lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <202205050919.496AC46@keescook> Date: Thu, 5 May 2022 09:23:23 -0700 From: Kees Cook <keescook@...omium.org> To: Jakub Kicinski <kuba@...nel.org> Cc: Raju Rangoju <rajur@...lsio.com>, kernel test robot <lkp@...el.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, stable@...r.kernel.org, Heiner Kallweit <hkallweit1@...il.com>, Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH] net: chelsio: cxgb4: Avoid potential negative array offset On Wed, May 04, 2022 at 08:13:58PM -0700, Jakub Kicinski wrote: > 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@...el.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? No, the build warning isn't in the current release, but I'm expecting to enable the next step of the FORTIFY work in the coming merge window. > > - 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? Yes, due to the enum, but you're right; this could just use min_t(uint... I'll respin! -- Kees Cook
Powered by blists - more mailing lists