[PATCH 0/7] net: Use scnprintf() for avoiding potential buffer overflow

[PATCH 0/7] net: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Hi,

here is a series of trivial patches just to convert suspicious
snprintf() usages with the more safer one, scnprintf().


Takashi

===

Takashi Iwai (7):
  net: caif: Use scnprintf() for avoiding potential buffer overflow
  i40e: Use scnprintf() for avoiding potential buffer overflow
  mlx4: Use scnprintf() for avoiding potential buffer overflow
  nfp: Use scnprintf() for avoiding potential buffer overflow
  ionic: Use scnprintf() for avoiding potential buffer overflow
  sfc: Use scnprintf() for avoiding potential buffer overflow
  netdevsim: Use scnprintf() for avoiding potential buffer overflow

 drivers/net/caif/caif_spi.c                        | 36 +++++++++++-----------
 drivers/net/ethernet/intel/i40e/i40e_main.c        | 24 +++++++--------
 drivers/net/ethernet/mellanox/mlx4/mcg.c           | 24 +++++++--------
 .../ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c  |  6 ++--
 drivers/net/ethernet/pensando/ionic/ionic_lif.c    | 12 ++++----
 drivers/net/ethernet/sfc/mcdi.c                    | 12 ++++----
 drivers/net/netdevsim/ipsec.c                      |  8 ++---
 7 files changed, 61 insertions(+), 61 deletions(-)

-- 
2.16.4

[PATCH 1/7] net: caif: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/caif/caif_spi.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/caif/caif_spi.c b/drivers/net/caif/caif_spi.c
index 8e81bdf98ac6..5457f2c5fd07 100644
--- a/drivers/net/caif/caif_spi.c
+++ b/drivers/net/caif/caif_spi.c
@@ -141,28 +141,28 @@ static ssize_t dbgfs_state(struct file *file, char __user *user_buf,
 		return 0;
 
 	/* Print out debug information. */
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"CAIF SPI debug information:\n");
 
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len), FLAVOR);
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len), FLAVOR);
 
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"STATE: %d\n", cfspi->dbg_state);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Previous CMD: 0x%x\n", cfspi->pcmd);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Current CMD: 0x%x\n", cfspi->cmd);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Previous TX len: %d\n", cfspi->tx_ppck_len);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Previous RX len: %d\n", cfspi->rx_ppck_len);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Current TX len: %d\n", cfspi->tx_cpck_len);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Current RX len: %d\n", cfspi->rx_cpck_len);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Next TX len: %d\n", cfspi->tx_npck_len);
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Next RX len: %d\n", cfspi->rx_npck_len);
 
 	if (len > DEBUGFS_BUF_SIZE)
@@ -180,23 +180,23 @@ static ssize_t print_frame(char *buf, size_t size, char *frm,
 	int len = 0;
 	int i;
 	for (i = 0; i < count; i++) {
-		len += snprintf((buf + len), (size - len),
+		len += scnprintf((buf + len), (size - len),
 					"[0x" BYTE_HEX_FMT "]",
 					frm[i]);
 		if ((i == cut) && (count > (cut * 2))) {
 			/* Fast forward. */
 			i = count - cut;
-			len += snprintf((buf + len), (size - len),
+			len += scnprintf((buf + len), (size - len),
 					"--- %zu bytes skipped ---\n",
 					count - (cut * 2));
 		}
 
 		if ((!(i % 10)) && i) {
-			len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+			len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 					"\n");
 		}
 	}
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len), "\n");
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len), "\n");
 	return len;
 }
 
@@ -214,17 +214,17 @@ static ssize_t dbgfs_frame(struct file *file, char __user *user_buf,
 		return 0;
 
 	/* Print out debug information. */
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Current frame:\n");
 
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Tx data (Len: %d):\n", cfspi->tx_cpck_len);
 
 	len += print_frame((buf + len), (DEBUGFS_BUF_SIZE - len),
 			   cfspi->xfer.va_tx[0],
 			   (cfspi->tx_cpck_len + SPI_CMD_SZ), 100);
 
-	len += snprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
+	len += scnprintf((buf + len), (DEBUGFS_BUF_SIZE - len),
 			"Rx data (Len: %d):\n", cfspi->rx_cpck_len);
 
 	len += print_frame((buf + len), (DEBUGFS_BUF_SIZE - len),
-- 
2.16.4

[PATCH 2/7] i40e: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 8c3e753bfb9d..ff431c13f858 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14478,29 +14478,29 @@ static void i40e_print_features(struct i40e_pf *pf)
 
 	i = snprintf(buf, INFO_STRING_LEN, "Features: PF-id[%d]", hw->pf_id);
 #ifdef CONFIG_PCI_IOV
-	i += snprintf(&buf[i], REMAIN(i), " VFs: %d", pf->num_req_vfs);
+	i += scnprintf(&buf[i], REMAIN(i), " VFs: %d", pf->num_req_vfs);
 #endif
-	i += snprintf(&buf[i], REMAIN(i), " VSIs: %d QP: %d",
+	i += scnprintf(&buf[i], REMAIN(i), " VSIs: %d QP: %d",
 		      pf->hw.func_caps.num_vsis,
 		      pf->vsi[pf->lan_vsi]->num_queue_pairs);
 	if (pf->flags & I40E_FLAG_RSS_ENABLED)
-		i += snprintf(&buf[i], REMAIN(i), " RSS");
+		i += scnprintf(&buf[i], REMAIN(i), " RSS");
 	if (pf->flags & I40E_FLAG_FD_ATR_ENABLED)
-		i += snprintf(&buf[i], REMAIN(i), " FD_ATR");
+		i += scnprintf(&buf[i], REMAIN(i), " FD_ATR");
 	if (pf->flags & I40E_FLAG_FD_SB_ENABLED) {
-		i += snprintf(&buf[i], REMAIN(i), " FD_SB");
-		i += snprintf(&buf[i], REMAIN(i), " NTUPLE");
+		i += scnprintf(&buf[i], REMAIN(i), " FD_SB");
+		i += scnprintf(&buf[i], REMAIN(i), " NTUPLE");
 	}
 	if (pf->flags & I40E_FLAG_DCB_CAPABLE)
-		i += snprintf(&buf[i], REMAIN(i), " DCB");
-	i += snprintf(&buf[i], REMAIN(i), " VxLAN");
-	i += snprintf(&buf[i], REMAIN(i), " Geneve");
+		i += scnprintf(&buf[i], REMAIN(i), " DCB");
+	i += scnprintf(&buf[i], REMAIN(i), " VxLAN");
+	i += scnprintf(&buf[i], REMAIN(i), " Geneve");
 	if (pf->flags & I40E_FLAG_PTP)
-		i += snprintf(&buf[i], REMAIN(i), " PTP");
+		i += scnprintf(&buf[i], REMAIN(i), " PTP");
 	if (pf->flags & I40E_FLAG_VEB_MODE_ENABLED)
-		i += snprintf(&buf[i], REMAIN(i), " VEB");
+		i += scnprintf(&buf[i], REMAIN(i), " VEB");
 	else
-		i += snprintf(&buf[i], REMAIN(i), " VEPA");
+		i += scnprintf(&buf[i], REMAIN(i), " VEPA");
 
 	dev_info(&pf->pdev->dev, "%s\n", buf);
 	kfree(buf);
-- 
2.16.4

RE: [Intel-wired-lan] [PATCH 2/7] i40e: Use scnprintf() for avoiding potential buffer overflow

[Bowers, AndrewX]

-----Original Message-----
From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Takashi Iwai
Sent: Wednesday, March 11, 2020 1:38 AM
To: netdev@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org; David S . Miller <davem@davemloft.net>
Subject: [Intel-wired-lan] [PATCH 2/7] i40e: Use scnprintf() for avoiding potential buffer overflow

Since snprintf() returns the would-be-output size instead of the actual output size, the succeeding calls may go beyond the given buffer limit.  Fix it by replacing with scnprintf().

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[PATCH 3/7] mlx4: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/mellanox/mlx4/mcg.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/mcg.c b/drivers/net/ethernet/mellanox/mlx4/mcg.c
index 9c481823b3e8..c0217239122d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mcg.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mcg.c
@@ -906,32 +906,32 @@ static void mlx4_err_rule(struct mlx4_dev *dev, char *str,
 	int len = 0;
 
 	mlx4_err(dev, "%s", str);
-	len += snprintf(buf + len, BUF_SIZE - len,
+	len += scnprintf(buf + len, BUF_SIZE - len,
 			"port = %d prio = 0x%x qp = 0x%x ",
 			rule->port, rule->priority, rule->qpn);
 
 	list_for_each_entry(cur, &rule->list, list) {
 		switch (cur->id) {
 		case MLX4_NET_TRANS_RULE_ID_ETH:
-			len += snprintf(buf + len, BUF_SIZE - len,
+			len += scnprintf(buf + len, BUF_SIZE - len,
 					"dmac = %pM ", &cur->eth.dst_mac);
 			if (cur->eth.ether_type)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"ethertype = 0x%x ",
 						be16_to_cpu(cur->eth.ether_type));
 			if (cur->eth.vlan_id)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"vlan-id = %d ",
 						be16_to_cpu(cur->eth.vlan_id));
 			break;
 
 		case MLX4_NET_TRANS_RULE_ID_IPV4:
 			if (cur->ipv4.src_ip)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"src-ip = %pI4 ",
 						&cur->ipv4.src_ip);
 			if (cur->ipv4.dst_ip)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"dst-ip = %pI4 ",
 						&cur->ipv4.dst_ip);
 			break;
@@ -939,25 +939,25 @@ static void mlx4_err_rule(struct mlx4_dev *dev, char *str,
 		case MLX4_NET_TRANS_RULE_ID_TCP:
 		case MLX4_NET_TRANS_RULE_ID_UDP:
 			if (cur->tcp_udp.src_port)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"src-port = %d ",
 						be16_to_cpu(cur->tcp_udp.src_port));
 			if (cur->tcp_udp.dst_port)
-				len += snprintf(buf + len, BUF_SIZE - len,
+				len += scnprintf(buf + len, BUF_SIZE - len,
 						"dst-port = %d ",
 						be16_to_cpu(cur->tcp_udp.dst_port));
 			break;
 
 		case MLX4_NET_TRANS_RULE_ID_IB:
-			len += snprintf(buf + len, BUF_SIZE - len,
+			len += scnprintf(buf + len, BUF_SIZE - len,
 					"dst-gid = %pI6\n", cur->ib.dst_gid);
-			len += snprintf(buf + len, BUF_SIZE - len,
+			len += scnprintf(buf + len, BUF_SIZE - len,
 					"dst-gid-mask = %pI6\n",
 					cur->ib.dst_gid_msk);
 			break;
 
 		case MLX4_NET_TRANS_RULE_ID_VXLAN:
-			len += snprintf(buf + len, BUF_SIZE - len,
+			len += scnprintf(buf + len, BUF_SIZE - len,
 					"VNID = %d ", be32_to_cpu(cur->vxlan.vni));
 			break;
 		case MLX4_NET_TRANS_RULE_ID_IPV6:
@@ -967,7 +967,7 @@ static void mlx4_err_rule(struct mlx4_dev *dev, char *str,
 			break;
 		}
 	}
-	len += snprintf(buf + len, BUF_SIZE - len, "\n");
+	len += scnprintf(buf + len, BUF_SIZE - len, "\n");
 	mlx4_err(dev, "%s", buf);
 
 	if (len >= BUF_SIZE)
-- 
2.16.4

[PATCH 4/7] nfp: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Jakub Kicinski <kuba@kernel.org>
Cc: oss-drivers@netronome.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
index 8fde6c1f681b..cc311989e3d7 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
@@ -616,7 +616,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
 	if (bar->iomem) {
 		int pf;
 
-		msg += snprintf(msg, end - msg,	"0.0: General/MSI-X SRAM, ");
+		msg += scnprintf(msg, end - msg,	"0.0: General/MSI-X SRAM, ");
 		atomic_inc(&bar->refcnt);
 		bars_free--;
 
@@ -661,7 +661,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
 
 	/* Configure, and lock, BAR0.1 for PCIe XPB (MSI-X PBA) */
 	bar = &nfp->bar[1];
-	msg += snprintf(msg, end - msg, "0.1: PCIe XPB/MSI-X PBA, ");
+	msg += scnprintf(msg, end - msg, "0.1: PCIe XPB/MSI-X PBA, ");
 	atomic_inc(&bar->refcnt);
 	bars_free--;
 
@@ -680,7 +680,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
 		bar->iomem = ioremap(nfp_bar_resource_start(bar),
 					     nfp_bar_resource_len(bar));
 		if (bar->iomem) {
-			msg += snprintf(msg, end - msg,
+			msg += scnprintf(msg, end - msg,
 					"0.%d: Explicit%d, ", 4 + i, i);
 			atomic_inc(&bar->refcnt);
 			bars_free--;
-- 
2.16.4

Re: [oss-drivers] [PATCH 4/7] nfp: Use scnprintf() for avoiding potential buffer overflow

[Simon Horman]

On Wed, Mar 11, 2020 at 09:37:42AM +0100, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit.  Fix it by replacing with scnprintf().
> 
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: oss-drivers@netronome.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Reviewed-by: Simon Horman <simon.horman@netronome.com>

Re: [PATCH 4/7] nfp: Use scnprintf() for avoiding potential buffer overflow

[Jakub Kicinski]

On Wed, 11 Mar 2020 09:37:42 +0100 Takashi Iwai wrote:
> @@ -680,7 +680,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
>  		bar->iomem = ioremap(nfp_bar_resource_start(bar),
>  					     nfp_bar_resource_len(bar));
>  		if (bar->iomem) {
> -			msg += snprintf(msg, end - msg,
> +			msg += scnprintf(msg, end - msg,
>  					"0.%d: Explicit%d, ", 4 + i, i);

Thanks for the patches! One nit pick - please adjust the continuation
lines so it starts on the column after the opening bracket (other
patches have the same problem).

You can try running scripts/checkpatch --strict on your patches, it
should catch these.

[PATCH 5/7] ionic: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Jakub Kicinski <kuba@kernel.org>
Cc: oss-drivers@netronome.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c |  2 +-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c           | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
index cc311989e3d7..7d518999250d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
@@ -616,7 +616,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
 	if (bar->iomem) {
 		int pf;
 
-		msg += scnprintf(msg, end - msg,	"0.0: General/MSI-X SRAM, ");
+		msg += scnprintf(msg, end - msg, "0.0: General/MSI-X SRAM, ");
 		atomic_inc(&bar->refcnt);
 		bars_free--;
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index c2f5b691e0fa..09c776191edd 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -948,18 +948,18 @@ static void ionic_lif_rx_mode(struct ionic_lif *lif, unsigned int rx_mode)
 	int i;
 #define REMAIN(__x) (sizeof(buf) - (__x))
 
-	i = snprintf(buf, sizeof(buf), "rx_mode 0x%04x -> 0x%04x:",
+	i = scnprintf(buf, sizeof(buf), "rx_mode 0x%04x -> 0x%04x:",
 		     lif->rx_mode, rx_mode);
 	if (rx_mode & IONIC_RX_MODE_F_UNICAST)
-		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_UNICAST");
+		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_UNICAST");
 	if (rx_mode & IONIC_RX_MODE_F_MULTICAST)
-		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_MULTICAST");
+		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_MULTICAST");
 	if (rx_mode & IONIC_RX_MODE_F_BROADCAST)
-		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_BROADCAST");
+		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_BROADCAST");
 	if (rx_mode & IONIC_RX_MODE_F_PROMISC)
-		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_PROMISC");
+		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_PROMISC");
 	if (rx_mode & IONIC_RX_MODE_F_ALLMULTI)
-		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_ALLMULTI");
+		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_ALLMULTI");
 	netdev_dbg(lif->netdev, "lif%d %s\n", lif->index, buf);
 
 	err = ionic_adminq_post_wait(lif, &ctx);
-- 
2.16.4

Re: [oss-drivers] [PATCH 5/7] ionic: Use scnprintf() for avoiding potential buffer overflow

[Simon Horman]

On Wed, Mar 11, 2020 at 09:37:43AM +0100, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit.  Fix it by replacing with scnprintf().
> 
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: oss-drivers@netronome.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Reviewed-by: Simon Horman <simon.horman@netronome.com>

Re: [PATCH 5/7] ionic: Use scnprintf() for avoiding potential buffer overflow

[Shannon Nelson]

On 3/11/20 1:37 AM, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit.  Fix it by replacing with scnprintf().
>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: oss-drivers@netronome.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

For ionic:
Acked-by: Shannon Nelson <snelson@pensando.io>


> ---
>   drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c |  2 +-
>   drivers/net/ethernet/pensando/ionic/ionic_lif.c           | 12 ++++++------
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
> index cc311989e3d7..7d518999250d 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c
> @@ -616,7 +616,7 @@ static int enable_bars(struct nfp6000_pcie *nfp, u16 interface)
>   	if (bar->iomem) {
>   		int pf;
>   
> -		msg += scnprintf(msg, end - msg,	"0.0: General/MSI-X SRAM, ");
> +		msg += scnprintf(msg, end - msg, "0.0: General/MSI-X SRAM, ");
>   		atomic_inc(&bar->refcnt);
>   		bars_free--;
>   
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index c2f5b691e0fa..09c776191edd 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -948,18 +948,18 @@ static void ionic_lif_rx_mode(struct ionic_lif *lif, unsigned int rx_mode)
>   	int i;
>   #define REMAIN(__x) (sizeof(buf) - (__x))
>   
> -	i = snprintf(buf, sizeof(buf), "rx_mode 0x%04x -> 0x%04x:",
> +	i = scnprintf(buf, sizeof(buf), "rx_mode 0x%04x -> 0x%04x:",
>   		     lif->rx_mode, rx_mode);
>   	if (rx_mode & IONIC_RX_MODE_F_UNICAST)
> -		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_UNICAST");
> +		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_UNICAST");
>   	if (rx_mode & IONIC_RX_MODE_F_MULTICAST)
> -		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_MULTICAST");
> +		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_MULTICAST");
>   	if (rx_mode & IONIC_RX_MODE_F_BROADCAST)
> -		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_BROADCAST");
> +		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_BROADCAST");
>   	if (rx_mode & IONIC_RX_MODE_F_PROMISC)
> -		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_PROMISC");
> +		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_PROMISC");
>   	if (rx_mode & IONIC_RX_MODE_F_ALLMULTI)
> -		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_ALLMULTI");
> +		i += scnprintf(&buf[i], REMAIN(i), " RX_MODE_F_ALLMULTI");
>   	netdev_dbg(lif->netdev, "lif%d %s\n", lif->index, buf);
>   
>   	err = ionic_adminq_post_wait(lif, &ctx);

[PATCH 6/7] sfc: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Martin Habets <mhabets@solarflare.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ethernet/sfc/mcdi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
index 2713300343c7..ac978e24644f 100644
--- a/drivers/net/ethernet/sfc/mcdi.c
+++ b/drivers/net/ethernet/sfc/mcdi.c
@@ -212,11 +212,11 @@ static void efx_mcdi_send_request(struct efx_nic *efx, unsigned cmd,
 		 * progress on a NIC at any one time.  So no need for locking.
 		 */
 		for (i = 0; i < hdr_len / 4 && bytes < PAGE_SIZE; i++)
-			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
+			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
 					  " %08x", le32_to_cpu(hdr[i].u32[0]));
 
 		for (i = 0; i < inlen / 4 && bytes < PAGE_SIZE; i++)
-			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
+			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
 					  " %08x", le32_to_cpu(inbuf[i].u32[0]));
 
 		netif_info(efx, hw, efx->net_dev, "MCDI RPC REQ:%s\n", buf);
@@ -302,14 +302,14 @@ static void efx_mcdi_read_response_header(struct efx_nic *efx)
 		 */
 		for (i = 0; i < hdr_len && bytes < PAGE_SIZE; i++) {
 			efx->type->mcdi_read_response(efx, &hdr, (i * 4), 4);
-			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
+			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
 					  " %08x", le32_to_cpu(hdr.u32[0]));
 		}
 
 		for (i = 0; i < data_len && bytes < PAGE_SIZE; i++) {
 			efx->type->mcdi_read_response(efx, &hdr,
 					mcdi->resp_hdr_len + (i * 4), 4);
-			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
+			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
 					  " %08x", le32_to_cpu(hdr.u32[0]));
 		}
 
@@ -1417,7 +1417,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
 	}
 
 	ver_words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_OUT_VERSION);
-	offset = snprintf(buf, len, "%u.%u.%u.%u",
+	offset = scnprintf(buf, len, "%u.%u.%u.%u",
 			  le16_to_cpu(ver_words[0]), le16_to_cpu(ver_words[1]),
 			  le16_to_cpu(ver_words[2]), le16_to_cpu(ver_words[3]));
 
@@ -1427,7 +1427,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
 	if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0) {
 		struct efx_ef10_nic_data *nic_data = efx->nic_data;
 
-		offset += snprintf(buf + offset, len - offset, " rx%x tx%x",
+		offset += scnprintf(buf + offset, len - offset, " rx%x tx%x",
 				   nic_data->rx_dpcpu_fw_id,
 				   nic_data->tx_dpcpu_fw_id);
 
-- 
2.16.4

Re: [PATCH 6/7] sfc: Use scnprintf() for avoiding potential buffer overflow

[Martin Habets]

Hi Takashi,

Fix looks ok, but could you please fix the alignment of the subsequent lines as well?

Thanks,
Martin

On 11/03/2020 08:37, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit.  Fix it by replacing with scnprintf().
> 
> Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Martin Habets <mhabets@solarflare.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/net/ethernet/sfc/mcdi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
> index 2713300343c7..ac978e24644f 100644
> --- a/drivers/net/ethernet/sfc/mcdi.c
> +++ b/drivers/net/ethernet/sfc/mcdi.c
> @@ -212,11 +212,11 @@ static void efx_mcdi_send_request(struct efx_nic *efx, unsigned cmd,
>  		 * progress on a NIC at any one time.  So no need for locking.
>  		 */
>  		for (i = 0; i < hdr_len / 4 && bytes < PAGE_SIZE; i++)
> -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
>  					  " %08x", le32_to_cpu(hdr[i].u32[0]));
>  
>  		for (i = 0; i < inlen / 4 && bytes < PAGE_SIZE; i++)
> -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
>  					  " %08x", le32_to_cpu(inbuf[i].u32[0]));
>  
>  		netif_info(efx, hw, efx->net_dev, "MCDI RPC REQ:%s\n", buf);
> @@ -302,14 +302,14 @@ static void efx_mcdi_read_response_header(struct efx_nic *efx)
>  		 */
>  		for (i = 0; i < hdr_len && bytes < PAGE_SIZE; i++) {
>  			efx->type->mcdi_read_response(efx, &hdr, (i * 4), 4);
> -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
>  					  " %08x", le32_to_cpu(hdr.u32[0]));
>  		}
>  
>  		for (i = 0; i < data_len && bytes < PAGE_SIZE; i++) {
>  			efx->type->mcdi_read_response(efx, &hdr,
>  					mcdi->resp_hdr_len + (i * 4), 4);
> -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
>  					  " %08x", le32_to_cpu(hdr.u32[0]));
>  		}
>  
> @@ -1417,7 +1417,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
>  	}
>  
>  	ver_words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_OUT_VERSION);
> -	offset = snprintf(buf, len, "%u.%u.%u.%u",
> +	offset = scnprintf(buf, len, "%u.%u.%u.%u",
>  			  le16_to_cpu(ver_words[0]), le16_to_cpu(ver_words[1]),
>  			  le16_to_cpu(ver_words[2]), le16_to_cpu(ver_words[3]));
>  
> @@ -1427,7 +1427,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
>  	if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0) {
>  		struct efx_ef10_nic_data *nic_data = efx->nic_data;
>  
> -		offset += snprintf(buf + offset, len - offset, " rx%x tx%x",
> +		offset += scnprintf(buf + offset, len - offset, " rx%x tx%x",
>  				   nic_data->rx_dpcpu_fw_id,
>  				   nic_data->tx_dpcpu_fw_id);
>  
> 

Re: [PATCH 6/7] sfc: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

On Thu, 12 Mar 2020 10:53:05 +0100,
Martin Habets wrote:
> 
> Hi Takashi,
> 
> Fix looks ok, but could you please fix the alignment of the subsequent lines as well?

Yes, I'll respin with that in v2, as other people also asked for it.


thanks,

Takashi

> 
> Thanks,
> Martin
> 
> On 11/03/2020 08:37, Takashi Iwai wrote:
> > Since snprintf() returns the would-be-output size instead of the
> > actual output size, the succeeding calls may go beyond the given
> > buffer limit.  Fix it by replacing with scnprintf().
> > 
> > Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
> > Cc: Edward Cree <ecree@solarflare.com>
> > Cc: Martin Habets <mhabets@solarflare.com>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/net/ethernet/sfc/mcdi.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
> > index 2713300343c7..ac978e24644f 100644
> > --- a/drivers/net/ethernet/sfc/mcdi.c
> > +++ b/drivers/net/ethernet/sfc/mcdi.c
> > @@ -212,11 +212,11 @@ static void efx_mcdi_send_request(struct efx_nic *efx, unsigned cmd,
> >  		 * progress on a NIC at any one time.  So no need for locking.
> >  		 */
> >  		for (i = 0; i < hdr_len / 4 && bytes < PAGE_SIZE; i++)
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr[i].u32[0]));
> >  
> >  		for (i = 0; i < inlen / 4 && bytes < PAGE_SIZE; i++)
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(inbuf[i].u32[0]));
> >  
> >  		netif_info(efx, hw, efx->net_dev, "MCDI RPC REQ:%s\n", buf);
> > @@ -302,14 +302,14 @@ static void efx_mcdi_read_response_header(struct efx_nic *efx)
> >  		 */
> >  		for (i = 0; i < hdr_len && bytes < PAGE_SIZE; i++) {
> >  			efx->type->mcdi_read_response(efx, &hdr, (i * 4), 4);
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr.u32[0]));
> >  		}
> >  
> >  		for (i = 0; i < data_len && bytes < PAGE_SIZE; i++) {
> >  			efx->type->mcdi_read_response(efx, &hdr,
> >  					mcdi->resp_hdr_len + (i * 4), 4);
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr.u32[0]));
> >  		}
> >  
> > @@ -1417,7 +1417,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
> >  	}
> >  
> >  	ver_words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_OUT_VERSION);
> > -	offset = snprintf(buf, len, "%u.%u.%u.%u",
> > +	offset = scnprintf(buf, len, "%u.%u.%u.%u",
> >  			  le16_to_cpu(ver_words[0]), le16_to_cpu(ver_words[1]),
> >  			  le16_to_cpu(ver_words[2]), le16_to_cpu(ver_words[3]));
> >  
> > @@ -1427,7 +1427,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
> >  	if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0) {
> >  		struct efx_ef10_nic_data *nic_data = efx->nic_data;
> >  
> > -		offset += snprintf(buf + offset, len - offset, " rx%x tx%x",
> > +		offset += scnprintf(buf + offset, len - offset, " rx%x tx%x",
> >  				   nic_data->rx_dpcpu_fw_id,
> >  				   nic_data->tx_dpcpu_fw_id);
> >  
> > 
> 

[PATCH 7/7] netdevsim: Use scnprintf() for avoiding potential buffer overflow

[Takashi Iwai]

Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit.  Fix it by replacing with scnprintf().

Cc: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/netdevsim/ipsec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/ipsec.c b/drivers/net/netdevsim/ipsec.c
index e27fc1a4516d..3281ce3d6c70 100644
--- a/drivers/net/netdevsim/ipsec.c
+++ b/drivers/net/netdevsim/ipsec.c
@@ -29,7 +29,7 @@ static ssize_t nsim_dbg_netdev_ops_read(struct file *filp,
 		return -ENOMEM;
 
 	p = buf;
-	p += snprintf(p, bufsize - (p - buf),
+	p += scnprintf(p, bufsize - (p - buf),
 		      "SA count=%u tx=%u\n",
 		      ipsec->count, ipsec->tx);
 
@@ -39,15 +39,15 @@ static ssize_t nsim_dbg_netdev_ops_read(struct file *filp,
 		if (!sap->used)
 			continue;
 
-		p += snprintf(p, bufsize - (p - buf),
+		p += scnprintf(p, bufsize - (p - buf),
 			      "sa[%i] %cx ipaddr=0x%08x %08x %08x %08x\n",
 			      i, (sap->rx ? 'r' : 't'), sap->ipaddr[0],
 			      sap->ipaddr[1], sap->ipaddr[2], sap->ipaddr[3]);
-		p += snprintf(p, bufsize - (p - buf),
+		p += scnprintf(p, bufsize - (p - buf),
 			      "sa[%i]    spi=0x%08x proto=0x%x salt=0x%08x crypt=%d\n",
 			      i, be32_to_cpu(sap->xs->id.spi),
 			      sap->xs->id.proto, sap->salt, sap->crypt);
-		p += snprintf(p, bufsize - (p - buf),
+		p += scnprintf(p, bufsize - (p - buf),
 			      "sa[%i]    key=0x%08x %08x %08x %08x\n",
 			      i, sap->key[0], sap->key[1],
 			      sap->key[2], sap->key[3]);
-- 
2.16.4