2019-11-18 17:09:53

by Jonathan Toppins

Subject: [PATCH net-next] bnx2x: initialize ethtool info->fw_version before use

If the info->fw_version has garbage in the buffer this can lead to a BUG()
being generated in strlcat() due to the use of strlen(). Initialize the
buffer before use.

The use of a systemtap script can demonstrate the problem by injecting
garbage into fw_version:

[root@localhost ~]# cat net-info.stp
//
// compile and run with command
//# stap -g net-info.stp
//
probe begin { printf("net-info version 01.01\n")}

function memset(msg:long) %{
memset((char*)STAP_ARG_msg,-1,196);
%}

probe module("bnx2x").function("bnx2x_get_drvinfo")
{
printf("In function\n");
memset(register("rsi"));
}

[root@localhost ~]# stap -g net-info.stp &
net-info version 01.01
[root@localhost ~]# ethtool -i eth1

Signed-off-by: Jonathan Toppins <[email protected]>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 4a0ba6801c9e..49b906396dcf 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -1111,6 +1111,8 @@ static void bnx2x_get_drvinfo(struct net_device *dev,
int ext_dev_info_offset;
u32 mbi;

+ info->fw_version[0] = 0;
+
strlcpy(info->driver, DRV_MODULE_NAME, sizeof(info->driver));
strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));

--
2.16.4

2019-11-18 22:22:56

by Jakub Kicinski

Subject: Re: [PATCH net-next] bnx2x: initialize ethtool info->fw_version before use

On Mon, 18 Nov 2019 12:07:53 -0500, Jonathan Toppins wrote:
> If the info->fw_version has garbage in the buffer this can lead to a BUG()
> being generated in strlcat() due to the use of strlen(). Initialize the
> buffer before use.
>
> The use of a systemtap script can demonstrate the problem by injecting
> garbage into fw_version:


> @@ -1111,6 +1111,8 @@ static void bnx2x_get_drvinfo(struct net_device *dev,
> int ext_dev_info_offset;
> u32 mbi;
>
> + info->fw_version[0] = 0;
> +
> strlcpy(info->driver, DRV_MODULE_NAME, sizeof(info->driver));
> strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));

I don't know systemtap, so it's very unclear what you're trying to fix
here. Setting random fields of info to 0 seems pointless as the entire
structure is zeroed before the call in ethtool_get_drvinfo().

Please explain.

2019-11-19 01:59:44

by David Miller

Subject: Re: [PATCH net-next] bnx2x: initialize ethtool info->fw_version before use

From: Jonathan Toppins <[email protected]>
Date: Mon, 18 Nov 2019 12:07:53 -0500

> probe begin { printf("net-info version 01.01\n")}
>
> function memset(msg:long) %{
> memset((char*)STAP_ARG_msg,-1,196);
> %}
>
> probe module("bnx2x").function("bnx2x_get_drvinfo")
> {
> printf("In function\n");
> memset(register("rsi"));

This makes no sense.

This function is called with the buffer cleared to zero.

You're scrambling something to simulate a "bug", but that will never
be scrambled in reality.