Patch "net/ncsi: Fix netlink major/minor version numbers" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    net/ncsi: Fix netlink major/minor version numbers

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     net-ncsi-fix-netlink-major-minor-version-numbers.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 337a17ed08e7fc149f67ac9766213ada2fb06078
Author: Peter Delevoryas <peter@xxxxxxx>
Date:   Tue Nov 14 10:07:34 2023 -0600

    net/ncsi: Fix netlink major/minor version numbers
    
    [ Upstream commit 3084b58bfd0b9e4b5e034f31f31b42977db35f12 ]
    
    The netlink interface for major and minor version numbers doesn't actually
    return the major and minor version numbers.
    
    It reports a u32 that contains the (major, minor, update, alpha1)
    components as the major version number, and then alpha2 as the minor
    version number.
    
    For whatever reason, the u32 byte order was reversed (ntohl): maybe it was
    assumed that the encoded value was a single big-endian u32, and alpha2 was
    the minor version.
    
    The correct way to get the supported NC-SI version from the network
    controller is to parse the Get Version ID response as described in 8.4.44
    of the NC-SI spec[1].
    
        Get Version ID Response Packet Format
    
                  Bits
                +--------+--------+--------+--------+
         Bytes  | 31..24 | 23..16 | 15..8  | 7..0   |
        +-------+--------+--------+--------+--------+
        | 0..15 | NC-SI Header                      |
        +-------+--------+--------+--------+--------+
        | 16..19| Response code   | Reason code     |
        +-------+--------+--------+--------+--------+
        |20..23 | Major  | Minor  | Update | Alpha1 |
        +-------+--------+--------+--------+--------+
        |24..27 |         reserved         | Alpha2 |
        +-------+--------+--------+--------+--------+
        |            .... other stuff ....          |
    
    The major, minor, and update fields are all binary-coded decimal (BCD)
    encoded [2]. The spec provides examples below the Get Version ID response
    format in section 8.4.44.1, but for practical purposes, this is an example
    from a live network card:
    
        root@bmc:~# ncsi-util 0x15
        NC-SI Command Response:
        cmd: GET_VERSION_ID(0x15)
        Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
        Payload length = 40
    
        20: 0xf1 0xf1 0xf0 0x00 <<<<<<<<< (major, minor, update, alpha1)
        24: 0x00 0x00 0x00 0x00 <<<<<<<<< (_, _, _, alpha2)
    
        28: 0x6d 0x6c 0x78 0x30
        32: 0x2e 0x31 0x00 0x00
        36: 0x00 0x00 0x00 0x00
        40: 0x16 0x1d 0x07 0xd2
        44: 0x10 0x1d 0x15 0xb3
        48: 0x00 0x17 0x15 0xb3
        52: 0x00 0x00 0x81 0x19
    
    This should be parsed as "1.1.0".
    
    "f" in the upper-nibble means to ignore it, contributing zero.
    
    If both nibbles are "f", I think the whole field is supposed to be ignored.
    Major and minor are "required", meaning they're not supposed to be "ff",
    but the update field is "optional" so I think it can be ff. I think the
    simplest thing to do is just set the major and minor to zero instead of
    juggling some conditional logic or something.
    
    bcd2bin() from "include/linux/bcd.h" seems to assume both nibbles are 0-9,
    so I've provided a custom BCD decoding function.
    
    Alpha1 and alpha2 are ISO/IEC 8859-1 encoded, which just means ASCII
    characters as far as I can tell, although the full encoding table for
    non-alphabetic characters is slightly different (I think).
    
    I imagine the alpha fields are just supposed to be alphabetic characters,
    but I haven't seen any network cards actually report a non-zero value for
    either.
    
    If people wrote software against this netlink behavior, and were parsing
    the major and minor versions themselves from the u32, then this would
    definitely break their code.
    
    [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf
    [2] https://en.wikipedia.org/wiki/Binary-coded_decimal
    [2] https://en.wikipedia.org/wiki/ISO/IEC_8859-1
    
    Signed-off-by: Peter Delevoryas <peter@xxxxxxx>
    Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 03757e76bb6b..374412ed780b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -105,8 +105,11 @@ enum {
 
 
 struct ncsi_channel_version {
-	u32 version;		/* Supported BCD encoded NCSI version */
-	u32 alpha2;		/* Supported BCD encoded NCSI version */
+	u8   major;		/* NCSI version major */
+	u8   minor;		/* NCSI version minor */
+	u8   update;		/* NCSI version update */
+	char alpha1;		/* NCSI version alpha1 */
+	char alpha2;		/* NCSI version alpha2 */
 	u8  fw_name[12];	/* Firmware name string                */
 	u32 fw_version;		/* Firmware version                   */
 	u16 pci_ids[4];		/* PCI identification                 */
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index d27f4eccce6d..fe681680b5d9 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -71,8 +71,8 @@ static int ncsi_write_channel_info(struct sk_buff *skb,
 	if (nc == nc->package->preferred_channel)
 		nla_put_flag(skb, NCSI_CHANNEL_ATTR_FORCED);
 
-	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MAJOR, nc->version.version);
-	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MINOR, nc->version.alpha2);
+	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MAJOR, nc->version.major);
+	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MINOR, nc->version.minor);
 	nla_put_string(skb, NCSI_CHANNEL_ATTR_VERSION_STR, nc->version.fw_name);
 
 	vid_nest = nla_nest_start_noflag(skb, NCSI_CHANNEL_ATTR_VLAN_LIST);
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index ba66c7dc3a21..c9d1da34dc4d 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -197,9 +197,12 @@ struct ncsi_rsp_gls_pkt {
 /* Get Version ID */
 struct ncsi_rsp_gvi_pkt {
 	struct ncsi_rsp_pkt_hdr rsp;          /* Response header */
-	__be32                  ncsi_version; /* NCSI version    */
+	unsigned char           major;        /* NCSI version major */
+	unsigned char           minor;        /* NCSI version minor */
+	unsigned char           update;       /* NCSI version update */
+	unsigned char           alpha1;       /* NCSI version alpha1 */
 	unsigned char           reserved[3];  /* Reserved        */
-	unsigned char           alpha2;       /* NCSI version    */
+	unsigned char           alpha2;       /* NCSI version alpha2 */
 	unsigned char           fw_name[12];  /* f/w name string */
 	__be32                  fw_version;   /* f/w version     */
 	__be16                  pci_ids[4];   /* PCI IDs         */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 069c2659074b..480e80e3c283 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -19,6 +19,19 @@
 #include "ncsi-pkt.h"
 #include "ncsi-netlink.h"
 
+/* Nibbles within [0xA, 0xF] add zero "0" to the returned value.
+ * Optional fields (encoded as 0xFF) will default to zero.
+ */
+static u8 decode_bcd_u8(u8 x)
+{
+	int lo = x & 0xF;
+	int hi = x >> 4;
+
+	lo = lo < 0xA ? lo : 0;
+	hi = hi < 0xA ? hi : 0;
+	return lo + hi * 10;
+}
+
 static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 				 unsigned short payload)
 {
@@ -755,9 +768,18 @@ static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
 	if (!nc)
 		return -ENODEV;
 
-	/* Update to channel's version info */
+	/* Update channel's version info
+	 *
+	 * Major, minor, and update fields are supposed to be
+	 * unsigned integers encoded as packed BCD.
+	 *
+	 * Alpha1 and alpha2 are ISO/IEC 8859-1 characters.
+	 */
 	ncv = &nc->version;
-	ncv->version = ntohl(rsp->ncsi_version);
+	ncv->major = decode_bcd_u8(rsp->major);
+	ncv->minor = decode_bcd_u8(rsp->minor);
+	ncv->update = decode_bcd_u8(rsp->update);
+	ncv->alpha1 = rsp->alpha1;
 	ncv->alpha2 = rsp->alpha2;
 	memcpy(ncv->fw_name, rsp->fw_name, 12);
 	ncv->fw_version = ntohl(rsp->fw_version);




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux