Skip to contentSkip to conversation

ibmvnic: Use kernel helpers for hex dumps

PATCH Applied Nick Child 3 participants 3 messages Mar 20, 2025 – Mar 25, 2025

Subsystems (4)
Networking Network drivers PowerPC IBM Power sriov Virtual NIC Device Driver
lore
Share & tools
lore

Nick Child

author reviewer
#0 lore raw

PATCH 1/1 ibmvnic: Use kernel helpers for hex dumps

Previously, when the driver was printing hex dumps, the buffer was cast to an 8 byte long and printed using string formatters. If the buffer size was not a multiple of 8 then a read buffer overflow was possible.

Therefore, create a new ibmvnic function that loops over a buffer and calls hex_dump_to_buffer instead.

This patch address KASAN reports like the one below:
ibmvnic 30000003 env3: Login Buffer:
ibmvnic 30000003 env3: 01000000af000000 <...>
ibmvnic 30000003 env3: 2e6d62692e736261 ibmvnic 30000003 env3: 65050003006d6f63
==================================================================

  BUG: KASAN: slab-out-of-bounds in ibmvnic_login+0xacc/0xffc [ibmvnic]
  Read of size 8 at addr c0000001331a9aa8 by task ip/17681
  <...>
  Allocated by task 17681:
  <...>
  ibmvnic_login+0x2f0/0xffc [ibmvnic]
  ibmvnic_open+0x148/0x308 [ibmvnic]
  __dev_open+0x1ac/0x304
  <...>
  The buggy address is located 168 bytes inside of
                allocated 175-byte region [c0000001331a9a00, c0000001331a9aaf)
  <...>
  =================================================================
  ibmvnic 30000003 env3: 000000000033766e
Fixes 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
Signed-off-by Nick Child <[email protected]>
Reviewed-by Dave Marquardt <[email protected]>

---
This patch obsoletes my work to define a for_each macro in printk.h [1]. It was determined the pitfalls outweighed the benefits of the code cleanup.

Side question, is net the correct mailing list even if the bug being addressed was introduced long before the current release? Or is net-next more appropriate? Does bug severity play any part in this or does anything with a Fixes tag go to net? netdev-FAQ implies all fixes go into net but previous mailing list entries seem to vary. Thanks

[1] https://lore.kernel.org/lkml/[email protected]/

 drivers/net/ethernet/ibm/ibmvnic.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)
drivers/net/ethernet/ibm/ibmvnic.c +18 -12
index 0676fc547b6f..480606d1245e 100644
@@ -4829,6 +4829,18 @@ static void vnic_add_client_data(struct ibmvnic_adapter *adapter,
 	strscpy(vlcd->name, adapter->netdev->name, len);
 }
 
+static void ibmvnic_print_hex_dump(struct net_device *dev, void *buf,
+				   size_t len)
+{
+	unsigned char hex_str[16 * 3];
+
+	for (size_t i = 0; i < len; i += 16) {
+		hex_dump_to_buffer((unsigned char *)buf + i, len - i, 16, 8,
+				   hex_str, sizeof(hex_str), false);
+		netdev_dbg(dev, "%s\n", hex_str);
+	}
+}
+
 static int send_login(struct ibmvnic_adapter *adapter)
 {
 	struct ibmvnic_login_rsp_buffer *login_rsp_buffer;

@@ -4939,10 +4951,8 @@ static int send_login(struct ibmvnic_adapter *adapter)
 	vnic_add_client_data(adapter, vlcd);
 
 	netdev_dbg(adapter->netdev, "Login Buffer:\n");
-	for (i = 0; i < (adapter->login_buf_sz - 1) / 8 + 1; i++) {
-		netdev_dbg(adapter->netdev, "%016lx\n",
-			   ((unsigned long *)(adapter->login_buf))[i]);
-	}
+	ibmvnic_print_hex_dump(adapter->netdev, adapter->login_buf,
+			       adapter->login_buf_sz);
 
 	memset(&crq, 0, sizeof(crq));
 	crq.login.first = IBMVNIC_CRQ_CMD;

@@ -5319,15 +5329,13 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 {
 	struct device *dev = &adapter->vdev->dev;
 	struct ibmvnic_query_ip_offload_buffer *buf = &adapter->ip_offload_buf;
-	int i;
 
 	dma_unmap_single(dev, adapter->ip_offload_tok,
 			 sizeof(adapter->ip_offload_buf), DMA_FROM_DEVICE);
 
 	netdev_dbg(adapter->netdev, "Query IP Offload Buffer:\n");
-	for (i = 0; i < (sizeof(adapter->ip_offload_buf) - 1) / 8 + 1; i++)
-		netdev_dbg(adapter->netdev, "%016lx\n",
-			   ((unsigned long *)(buf))[i]);
+	ibmvnic_print_hex_dump(adapter->netdev, buf,
+			       sizeof(adapter->ip_offload_buf));
 
 	netdev_dbg(adapter->netdev, "ipv4_chksum = %d\n", buf->ipv4_chksum);
 	netdev_dbg(adapter->netdev, "ipv6_chksum = %d\n", buf->ipv6_chksum);

@@ -5558,10 +5566,8 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq,
 	netdev->mtu = adapter->req_mtu - ETH_HLEN;
 
 	netdev_dbg(adapter->netdev, "Login Response Buffer:\n");
-	for (i = 0; i < (adapter->login_rsp_buf_sz - 1) / 8 + 1; i++) {
-		netdev_dbg(adapter->netdev, "%016lx\n",
-			   ((unsigned long *)(adapter->login_rsp_buf))[i]);
-	}
+	ibmvnic_print_hex_dump(netdev, adapter->login_rsp_buf,
+			       adapter->login_rsp_buf_sz);
 
 	/* Sanity checks */
 	if (login->num_txcomp_subcrqs != login_rsp->num_txsubm_subcrqs ||
Signature · 1 line
2.48.1
reviewer Reviewed-by on v1 1/1
#1 lore raw
Quoted 29 lines · Nick Child

Previously, when the driver was printing hex dumps, the buffer was cast to an 8 byte long and printed using string formatters. If the buffer size was not a multiple of 8 then a read buffer overflow was possible.

Therefore, create a new ibmvnic function that loops over a buffer and calls hex_dump_to_buffer instead.

This patch address KASAN reports like the one below:
ibmvnic 30000003 env3: Login Buffer:
ibmvnic 30000003 env3: 01000000af000000 <...>
ibmvnic 30000003 env3: 2e6d62692e736261 ibmvnic 30000003 env3: 65050003006d6f63
==================================================================

  BUG: KASAN: slab-out-of-bounds in ibmvnic_login+0xacc/0xffc [ibmvnic]
  Read of size 8 at addr c0000001331a9aa8 by task ip/17681
  <...>
  Allocated by task 17681:
  <...>
  ibmvnic_login+0x2f0/0xffc [ibmvnic]
  ibmvnic_open+0x148/0x308 [ibmvnic]
  __dev_open+0x1ac/0x304
  <...>
  The buggy address is located 168 bytes inside of
                allocated 175-byte region [c0000001331a9a00, c0000001331a9aaf)
  <...>
  =================================================================
  ibmvnic 30000003 env3: 000000000033766e
Quoted 3 lines
Fixes 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
Signed-off-by Nick Child <[email protected]>
Reviewed-by Dave Marquardt <[email protected]>
Reviewed-by Simon Horman <[email protected]>
Quoted 10 lines

---
This patch obsoletes my work to define a for_each macro in printk.h [1]. It was determined the pitfalls outweighed the benefits of the code cleanup.

Side question, is net the correct mailing list even if the bug being addressed was introduced long before the current release? Or is net-next more appropriate? Does bug severity play any part in this or does anything with a Fixes tag go to net? netdev-FAQ implies all fixes go into net but previous mailing list entries seem to vary. Thanks

My view is that net is the correct target for bug fixes for Networking code provided the bug is present in net (which would be the case here).

CI Applied
#2 lore raw
In reply to Nick Child
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 8 lines · On Thu, 20 Mar 2025 16:29:51 -0500 you wrote:

Previously, when the driver was printing hex dumps, the buffer was cast to an 8 byte long and printed using string formatters. If the buffer size was not a multiple of 8 then a read buffer overflow was possible.

Therefore, create a new ibmvnic function that loops over a buffer and calls hex_dump_to_buffer instead.

[...]

Here is the summary with links:
- [net] ibmvnic: Use kernel helpers for hex dumps

    https://git.kernel.org/netdev/net/c/d93a6caab5d7

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html