From 7ed716823fd02d84ea53cb61350bf14f248ebb8b Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Tue, 18 Aug 2020 09:51:22 +0200 Subject: [PATCH] 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 --- src/mds/MDSMap.cc | 4 +++- src/mds/PurgeQueue.cc | 8 ++++++-- src/messages/MMDSBeacon.h | 12 +++++++++--- src/messages/MMgrReport.h | 8 ++++++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/mds/MDSMap.cc b/src/mds/MDSMap.cc index b359573b90f21..95d10c9aebb27 100644 --- a/src/mds/MDSMap.cc +++ b/src/mds/MDSMap.cc @@ -577,7 +577,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 fc297f070fc7d..bdb81166f462d 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 += 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 03c7c80247266..e5d7589a49193 100644 --- a/src/messages/MMDSBeacon.h +++ b/src/messages/MMDSBeacon.h @@ -131,9 +131,13 @@ struct MDSHealthMetric void decode(ceph::buffer::list::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); @@ -273,7 +277,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 c1a55d880cf60..767ee775d22a5 100644 --- a/src/messages/MMgrReport.h +++ b/src/messages/MMgrReport.h @@ -61,12 +61,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); } -- 2.39.5