net: mctp: don't require received header reserved bits to be zero

[PATCH net-next v3] net: mctp: don't require received header reserved bits to be zero

Posted by wit_yuan 2 months ago

From: Yuan Zhaoming <yuanzm2@lenovo.com>

From the MCTP Base specification (DSP0236 v1.2.1), the first byte of
the MCTP header contains a 4 bit reserved field, and 4 bit version.

On our current receive path, we require those 4 reserved bits to be
zero, but the 9500-8i card is non-conformant, and may set these
reserved bits.

DSP0236 states that the reserved bits must be written as zero, and
ignored when read. While the device might not conform to the former,
we should accept these message to conform to the latter.

Relax our check on the MCTP version byte to allow non-zero bits in the
reserved field.

Signed-off-by: Yuan Zhaoming <yuanzm2@lenovo.com>

---
v2: https://lore.kernel.org/netdev/20260410144339.0d1b289a@kernel.org/T/#t
v1: https://lore.kernel.org/netdev/ff147a3f0d27ef2aa6026cc86f9113d56a8c61ac.camel@codeconstruct.com.au/T/#t
---
 include/net/mctp.h | 3 +++
 net/mctp/route.c   | 8 ++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/net/mctp.h b/include/net/mctp.h
index e1e0a69..d8bf907 100644
--- a/include/net/mctp.h
+++ b/include/net/mctp.h
@@ -26,6 +26,9 @@ struct mctp_hdr {
 #define MCTP_VER_MIN	1
 #define MCTP_VER_MAX	1
 
+/* Definitions for ver field */
+#define MCTP_HDR_VER_MASK	GENMASK(3, 0)
+
 /* Definitions for flags_seq_tag field */
 #define MCTP_HDR_FLAG_SOM	BIT(7)
 #define MCTP_HDR_FLAG_EOM	BIT(6)
diff --git a/net/mctp/route.c b/net/mctp/route.c
index e69c6f7..62517c9 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -439,6 +439,7 @@ static int mctp_dst_input(struct mctp_dst *dst, struct sk_buff *skb)
 	struct mctp_hdr *mh;
 	unsigned int netid;
 	unsigned long f;
+	u8 ver;
 	u8 tag, flags;
 	int rc;
 
@@ -467,7 +468,8 @@ static int mctp_dst_input(struct mctp_dst *dst, struct sk_buff *skb)
 	netid = mctp_cb(skb)->net;
 	skb_pull(skb, sizeof(struct mctp_hdr));
 
-	if (mh->ver != 1)
+	ver = mh->ver & MCTP_HDR_VER_MASK;
+	if (ver < MCTP_VER_MIN || ver > MCTP_VER_MAX)
 		goto out;
 
 	flags = mh->flags_seq_tag & (MCTP_HDR_FLAG_SOM | MCTP_HDR_FLAG_EOM);
@@ -1316,6 +1318,7 @@ static int mctp_pkttype_receive(struct sk_buff *skb, struct net_device *dev,
 	struct mctp_skb_cb *cb;
 	struct mctp_dst dst;
 	struct mctp_hdr *mh;
+	u8 ver;
 	int rc;
 
 	rcu_read_lock();
@@ -1334,7 +1337,8 @@ static int mctp_pkttype_receive(struct sk_buff *skb, struct net_device *dev,
 
 	/* We have enough for a header; decode and route */
 	mh = mctp_hdr(skb);
-	if (mh->ver < MCTP_VER_MIN || mh->ver > MCTP_VER_MAX)
+	ver = mh->ver & MCTP_HDR_VER_MASK;
+	if (ver < MCTP_VER_MIN || ver > MCTP_VER_MAX)
 		goto err_drop;
 
 	/* source must be valid unicast or null; drop reserved ranges and
-- 
2.43.0

Re: [PATCH net-next v3] net: mctp: don't require received header reserved bits to be zero

Posted by Paolo Abeni 2 months ago

On 4/13/26 10:03 AM, wit_yuan wrote:
> From: Yuan Zhaoming <yuanzm2@lenovo.com>
> 
> From the MCTP Base specification (DSP0236 v1.2.1), the first byte of
> the MCTP header contains a 4 bit reserved field, and 4 bit version.
> 
> On our current receive path, we require those 4 reserved bits to be
> zero, but the 9500-8i card is non-conformant, and may set these
> reserved bits.
> 
> DSP0236 states that the reserved bits must be written as zero, and
> ignored when read. While the device might not conform to the former,
> we should accept these message to conform to the latter.
> 
> Relax our check on the MCTP version byte to allow non-zero bits in the
> reserved field.
> 
> Signed-off-by: Yuan Zhaoming <yuanzm2@lenovo.com>

The net-next tree is currently closed for the merge window, but IMHO
this change could be considered a fix. Please repost for 'net' and add a
suitable fixes tag.

> ---
> v2: https://lore.kernel.org/netdev/20260410144339.0d1b289a@kernel.org/T/#t
> v1: https://lore.kernel.org/netdev/ff147a3f0d27ef2aa6026cc86f9113d56a8c61ac.camel@codeconstruct.com.au/T/#t
> ---
>  include/net/mctp.h | 3 +++
>  net/mctp/route.c   | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/mctp.h b/include/net/mctp.h
> index e1e0a69..d8bf907 100644
> --- a/include/net/mctp.h
> +++ b/include/net/mctp.h
> @@ -26,6 +26,9 @@ struct mctp_hdr {
>  #define MCTP_VER_MIN	1
>  #define MCTP_VER_MAX	1
>  
> +/* Definitions for ver field */
> +#define MCTP_HDR_VER_MASK	GENMASK(3, 0)
> +
>  /* Definitions for flags_seq_tag field */
>  #define MCTP_HDR_FLAG_SOM	BIT(7)
>  #define MCTP_HDR_FLAG_EOM	BIT(6)
> diff --git a/net/mctp/route.c b/net/mctp/route.c
> index e69c6f7..62517c9 100644
> --- a/net/mctp/route.c
> +++ b/net/mctp/route.c
> @@ -439,6 +439,7 @@ static int mctp_dst_input(struct mctp_dst *dst, struct sk_buff *skb)
>  	struct mctp_hdr *mh;
>  	unsigned int netid;
>  	unsigned long f;
> +	u8 ver;
>  	u8 tag, flags;
>  	int rc;

Please respect the reverse christmas tree order above.

/P