| Message ID | f40bcf8cd93677c12bca1f06e74385c9a5f49819.1574096873.git.jtoppins@redhat.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] bnx2x: initialize ethtool info->fw_version before use | expand |
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.
From: Jonathan Toppins <jtoppins@redhat.com> 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.
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));
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 <jtoppins@redhat.com> --- drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 ++ 1 file changed, 2 insertions(+)