[PATCH net] pds_core: fix potential stack info leak in firmware version reporting

[PATCH net] pds_core: fix potential stack info leak in firmware version reporting

[Nikhil P. Rao]

The fw_version field in pds_core_fw_name_info is a fixed 32-byte array
that may not be null-terminated if firmware writes exactly 32
characters. When passed to devlink_info_version_stored_put(), this
could cause a read beyond the array boundary, potentially leaking
stack contents to userspace or causing a crash if the read crosses
into an unmapped page.

Null-terminate the firmware version string in place before passing it
to the devlink API.

Fixes: 45d76f492938 ("pds_core: set up device and adminq")
Assisted-by: Claude:claude-opus-4
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
---
 drivers/net/ethernet/amd/pds_core/devlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index b576be626a29..3f0e56b951bf 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -122,12 +122,14 @@ int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 
 	listlen = min(fw_list.num_fw_slots, ARRAY_SIZE(fw_list.fw_names));
 	for (i = 0; i < listlen; i++) {
+		char *fw_ver = fw_list.fw_names[i].fw_version;
+
 		if (i < ARRAY_SIZE(fw_slotnames))
 			strscpy(buf, fw_slotnames[i], sizeof(buf));
 		else
 			snprintf(buf, sizeof(buf), "fw.slot_%d", i);
-		err = devlink_info_version_stored_put(req, buf,
-						      fw_list.fw_names[i].fw_version);
+		fw_ver[sizeof(fw_list.fw_names[i].fw_version) - 1] = '\0';
+		err = devlink_info_version_stored_put(req, buf, fw_ver);
 		if (err)
 			return err;
 	}
-- 
2.43.0

Re: [PATCH net] pds_core: fix potential stack info leak in firmware version reporting

[Jakub Kicinski]

On Fri, 15 May 2026 21:29:06 +0000 Nikhil P. Rao wrote:
> The fw_version field in pds_core_fw_name_info is a fixed 32-byte array
> that may not be null-terminated if firmware writes exactly 32
> characters. When passed to devlink_info_version_stored_put(), this
> could cause a read beyond the array boundary, potentially leaking
> stack contents to userspace or causing a crash if the read crosses
> into an unmapped page.
> 
> Null-terminate the firmware version string in place before passing it
> to the devlink API.

This loses the last, presumably meaningful character of the FW version.
Are you saying that device should never send names this long and this
is defensive? Otherwise we could trivially memcpy() the name into a
buffer on the stack that has one extra byte.

Please fix or clarify the commit msg.
-- 
pw-bot: cr