netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: "David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>
Cc: Michael Chan <michael.chan@broadcom.com>, netdev@vger.kernel.org
Subject: [PATCH net-next 1/3] bnxt_en: check for fw_ver_str truncation
Date: Fri, 05 Jul 2024 12:26:47 +0100	[thread overview]
Message-ID: <20240705-bnxt-str-v1-1-bafc769ed89e@kernel.org> (raw)
In-Reply-To: <20240705-bnxt-str-v1-0-bafc769ed89e@kernel.org>

Given the sizes of the buffers involved, it is theoretically
possible for fw_ver_str to be truncated. Detect this and
stop ethtool initialisation if this occurs.

Flagged by gcc-14:

  .../bnxt_ethtool.c: In function 'bnxt_ethtool_init':
  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:4144:32: warning: '%s' directive output may be truncated writing up to 31 bytes into a region of size 26 [-Wformat-truncation=]
   4144 |                          "/pkg %s", buf);
        |                                ^~   ~~~
  In function 'bnxt_get_pkgver',
      inlined from 'bnxt_ethtool_init' at .../bnxt_ethtool.c:5056:3:
  .../bnxt_ethtool.c:4143:17: note: 'snprintf' output between 6 and 37 bytes into a destination of size 31
   4143 |                 snprintf(bp->fw_ver_str + len, FW_VER_STR_LEN - len - 1,
        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   4144 |                          "/pkg %s", buf);
        |                          ~~~~~~~~~~~~~~~

Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
It appears to me that size is underestimated by 1 byte -
it should be FW_VER_STR_LEN - offset rather than FW_VER_STR_LEN - offset - 1,
because the size argument to snprintf should include the space for the
trailing '\0'. But I have not changed that as it is separate from
the issue this patch addresses.
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index bf157f6cc042..5ccc3cc4ba7d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4132,17 +4132,23 @@ int bnxt_get_pkginfo(struct net_device *dev, char *ver, int size)
 	return rc;
 }
 
-static void bnxt_get_pkgver(struct net_device *dev)
+static int bnxt_get_pkgver(struct net_device *dev)
 {
 	struct bnxt *bp = netdev_priv(dev);
 	char buf[FW_VER_STR_LEN];
-	int len;
 
 	if (!bnxt_get_pkginfo(dev, buf, sizeof(buf))) {
-		len = strlen(bp->fw_ver_str);
-		snprintf(bp->fw_ver_str + len, FW_VER_STR_LEN - len - 1,
-			 "/pkg %s", buf);
+		int offset, size, rc;
+
+		offset = strlen(bp->fw_ver_str);
+		size = FW_VER_STR_LEN - offset - 1;
+
+		rc = snprintf(bp->fw_ver_str + offset, size, "/pkg %s", buf);
+		if (rc >= size)
+			return -E2BIG;
 	}
+
+	return 0;
 }
 
 static int bnxt_get_eeprom(struct net_device *dev,
@@ -5052,8 +5058,11 @@ void bnxt_ethtool_init(struct bnxt *bp)
 	struct net_device *dev = bp->dev;
 	int i, rc;
 
-	if (!(bp->fw_cap & BNXT_FW_CAP_PKG_VER))
-		bnxt_get_pkgver(dev);
+	if (!(bp->fw_cap & BNXT_FW_CAP_PKG_VER)) {
+		rc = bnxt_get_pkgver(dev);
+		if (rc)
+			return;
+	}
 
 	bp->num_tests = 0;
 	if (bp->hwrm_spec_code < 0x10704 || !BNXT_PF(bp))

-- 
2.43.0


  reply	other threads:[~2024-07-05 11:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05 11:26 [PATCH net-next 0/3] bnxt_en: address string truncation Simon Horman
2024-07-05 11:26 ` Simon Horman [this message]
2024-07-05 12:37   ` [PATCH net-next 1/3] bnxt_en: check for fw_ver_str truncation Przemek Kitszel
2024-07-05 16:06     ` Simon Horman
2024-07-05 17:03       ` Pavan Chebbi
2024-07-05 17:39         ` Michael Chan
2024-07-05 11:26 ` [PATCH net-next 2/3] bnxt_en: check for irq name truncation Simon Horman
2024-07-05 18:27   ` Michael Chan
2024-07-05 19:09     ` Simon Horman
2024-07-05 11:26 ` [PATCH net-next 3/3] bnxt_en: avoid truncation of per rx run debugfs filename Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240705-bnxt-str-v1-1-bafc769ed89e@kernel.org \
    --to=horms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).