From: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Date: Tue, 18 Aug 2020 07:51:22 +0000 (+0200)
Subject: messages,mds: Fix decoding of enum types on big-endian systems
X-Git-Tag: v14.2.12~21^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cf8ff54d208cef665d316cde3fc6d01a36f943ca;p=ceph.git

messages,mds: Fix decoding of enum types on big-endian systems

When a struct member that has enum type needs to be encoded or
decoded, we need to use an explicit integer type, since there
are no encode routines for the enum type.  (This is probably
to avoid introducing dependencies on implementation-defined
choices by the compiler to use a particular underlying type.)

This leads to code sequences along the lines of:
  encode((int32_t)state, bl);
and
  decode((int32_t&)(state), bl);

The encode line is actually fine, but the decode line is
incorrect on big-endian systems if the underlying type of
the enum differs from the explicitly chosen integer type.

This is because this performs in effect a pointer cast,
and will write the decoded int32_t value into the memory
backing the "state" member variable.  If the sizes differ,
the value is written into the wrong bytes of "state" on
big-endian systems.

This patch fixes the problem by decoding into an intermediate
variable of the integer type first, and then casting the result
while assigning to the struct member of enum type.

This bug showed up initially as invalid health-status values
causing Ceph daemon aborts on s390x.  I've tried to find and
fix all other instances of the same enum decode pattern as well.

Fixes: https://tracker.ceph.com/issues/47015

Signed-off-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
(cherry picked from commit 7ed716823fd02d84ea53cb61350bf14f248ebb8b)

Conflicts:
	src/mds/PurgeQueue.cc
- nautilus has "p.advance(pad_size)", instead of "p += pad_size",
  in the line immediately preceding the first change
---

diff --git a/src/mds/MDSMap.cc b/src/mds/MDSMap.cc
index 07fdbd318d61..27753c6ec0af 100644
--- a/src/mds/MDSMap.cc
+++ b/src/mds/MDSMap.cc
@@ -549,7 +549,9 @@ void MDSMap::mds_info_t::decode(bufferlist::const_iterator& bl)
   decode(name, bl);
   decode(rank, bl);
   decode(inc, bl);
-  decode((int32_t&)(state), bl);
+  int32_t raw_state;
+  decode(raw_state, bl);
+  state = (MDSMap::DaemonState)raw_state;
   decode(state_seq, bl);
   decode(addrs, bl);
   decode(laggy_since, bl);
diff --git a/src/mds/PurgeQueue.cc b/src/mds/PurgeQueue.cc
index f730e4b07605..b7db57aa2b46 100644
--- a/src/mds/PurgeQueue.cc
+++ b/src/mds/PurgeQueue.cc
@@ -65,7 +65,9 @@ void PurgeItem::decode(bufferlist::const_iterator &p)
       decode(stamp, p);
       decode(pad_size, p);
       p.advance(pad_size);
-      decode((uint8_t&)action, p);
+      uint8_t raw_action;
+      decode(raw_action, p);
+      action = (Action)raw_action;
       decode(ino, p);
       decode(size, p);
       decode(layout, p);
@@ -80,7 +82,9 @@ void PurgeItem::decode(bufferlist::const_iterator &p)
     }
   }
   if (!done) {
-    decode((uint8_t&)action, p);
+    uint8_t raw_action;
+    decode(raw_action, p);
+    action = (Action)raw_action;
     decode(ino, p);
     decode(size, p);
     decode(layout, p);
diff --git a/src/messages/MMDSBeacon.h b/src/messages/MMDSBeacon.h
index 24922ae33303..10b5ae46f3ca 100644
--- a/src/messages/MMDSBeacon.h
+++ b/src/messages/MMDSBeacon.h
@@ -131,9 +131,13 @@ struct MDSHealthMetric
 
   void decode(bufferlist::const_iterator& bl) {
     DECODE_START(1, bl);
-    decode((uint16_t&)type, bl);
+    uint16_t raw_type;
+    decode(raw_type, bl);
+    type = (mds_metric_t)raw_type;
     ceph_assert(type != MDS_HEALTH_NULL);
-    decode((uint8_t&)sev, bl);
+    uint8_t raw_sev;
+    decode(raw_sev, bl);
+    sev = (health_status_t)raw_sev;
     decode(message, bl);
     decode(metadata, bl);
     DECODE_FINISH(bl);
@@ -264,7 +268,9 @@ public:
     paxos_decode(p);
     decode(fsid, p);
     decode(global_id, p);
-    decode((__u32&)state, p);
+    __u32 raw_state;
+    decode(raw_state, p);
+    state = (MDSMap::DaemonState)raw_state;
     decode(seq, p);
     decode(name, p);
     {
diff --git a/src/messages/MMgrReport.h b/src/messages/MMgrReport.h
index 84c96e7f200f..e98871178d19 100644
--- a/src/messages/MMgrReport.h
+++ b/src/messages/MMgrReport.h
@@ -59,12 +59,16 @@ public:
     decode(path, p);
     decode(description, p);
     decode(nick, p);
-    decode((uint8_t&)type, p);
+    uint8_t raw_type;
+    decode(raw_type, p);
+    type = (enum perfcounter_type_d)raw_type;
     if (struct_v >= 2) {
       decode(priority, p);
     }
     if (struct_v >= 3) {
-      decode((uint8_t&)unit, p);
+      uint8_t raw_unit;
+      decode(raw_unit, p);
+      unit = (enum unit_t)raw_unit;
     }
     DECODE_FINISH(p);
   }