lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 23 Dec 2020 15:45:32 +0100
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     netdev@...r.kernel.org,
        Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc:     linux-kernel@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Rasmus Villemoes <rasmus.villemoes@...vas.dk>
Subject: [PATCH net 1/2] net: mrp: fix definitions of MRP test packets

Wireshark says that the MRP test packets cannot be decoded - and the
reason for that is that there's a two-byte hole filled with garbage
between the "transitions" and "timestamp" members.

So Wireshark decodes the two garbage bytes and the top two bytes of
the timestamp written by the kernel as the timestamp value (which thus
fluctuates wildly), and interprets the lower two bytes of the
timestamp as a new (type, length) pair, which is of course broken.

While my copy of the MRP standard is still under way [*], I cannot
imagine the standard specifying a two-byte hole here, and whoever
wrote the Wireshark decoding code seems to agree with that.

The struct definitions live under include/uapi/, but they are not
really part of any kernel<->userspace API/ABI, so fixing the
definitions by adding the packed attribute should not cause any
compatibility issues.

The remaining on-the-wire packet formats likely also don't contain
holes, but pahole and manual inspection says the current definitions
suffice. So adding the packed attribute to those is not strictly
needed, but might be done for good measure.

[*] I will never understand how something hidden behind a +1000$
paywall can be called a standard.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@...vas.dk>
---
 include/uapi/linux/mrp_bridge.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
index 6aeb13ef0b1e..d1d0cf65916d 100644
--- a/include/uapi/linux/mrp_bridge.h
+++ b/include/uapi/linux/mrp_bridge.h
@@ -96,7 +96,7 @@ struct br_mrp_ring_test_hdr {
 	__be16 state;
 	__be16 transitions;
 	__be32 timestamp;
-};
+} __attribute__((__packed__));
 
 struct br_mrp_ring_topo_hdr {
 	__be16 prio;
@@ -141,7 +141,7 @@ struct br_mrp_in_test_hdr {
 	__be16 state;
 	__be16 transitions;
 	__be32 timestamp;
-};
+} __attribute__((__packed__));
 
 struct br_mrp_in_topo_hdr {
 	__u8 sa[ETH_ALEN];
-- 
2.23.0

Powered by blists - more mailing lists