lkml.org 
[lkml]   [2022]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] net: chelsio: cxgb4: Avoid potential negative array offset
On Tue,  3 May 2022 07:44:25 -0700 Kees Cook wrote:
> Using min_t(int, ...) as a potential array index implies to the compiler
> that negative offsets should be allowed. This is not the case, though.
> Replace min_t() with clamp_t(). Fixes the following warning exposed
> under future CONFIG_FORTIFY_SOURCE improvements:

> Additionally remove needless cast from u8[] to char * in last strim()
> call.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/202205031926.FVP7epJM-lkp@intel.com
> Fixes: fc9279298e3a ("cxgb4: Search VPD with pci_vpd_find_ro_info_keyword()")
> Fixes: 24c521f81c30 ("cxgb4: Use pci_vpd_find_id_string() to find VPD ID string")

Is it needed in the current release?

> Cc: Raju Rangoju <rajur@chelsio.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: netdev@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> index e7b4e3ed056c..f119ec7323e5 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> @@ -2793,14 +2793,14 @@ int t4_get_raw_vpd_params(struct adapter *adapter, struct vpd_params *p)
> goto out;
> na = ret;
>
> - memcpy(p->id, vpd + id, min_t(int, id_len, ID_LEN));
> + memcpy(p->id, vpd + id, clamp_t(int, id_len, 0, ID_LEN));

The typing is needed because of the enum, right? The variable is
unsigned, seems a little strange to use clamp(int, ..., 0, constant)
min(unsigned int, ..., constant) will be equivalent with fewer branches.
Is it just me?

> strim(p->id);
> - memcpy(p->sn, vpd + sn, min_t(int, sn_len, SERNUM_LEN));
> + memcpy(p->sn, vpd + sn, clamp_t(int, sn_len, 0, SERNUM_LEN));
> strim(p->sn);
> - memcpy(p->pn, vpd + pn, min_t(int, pn_len, PN_LEN));
> + memcpy(p->pn, vpd + pn, clamp_t(int, pn_len, 0, PN_LEN));
> strim(p->pn);
> - memcpy(p->na, vpd + na, min_t(int, na_len, MACADDR_LEN));
> - strim((char *)p->na);
> + memcpy(p->na, vpd + na, clamp_t(int, na_len, 0, MACADDR_LEN));
> + strim(p->na);
>
> out:
> vfree(vpd);

\
 
 \ /
  Last update: 2022-05-05 05:14    [W:0.101 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site