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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 2 Jun 2021 13:52:59 -0700 From: Kees Cook <keescook@...omium.org> To: Jay Vosburgh <jay.vosburgh@...onical.com> Cc: kernel test robot <lkp@...el.com>, Veaceslav Falico <vfalico@...il.com>, Andy Gospodarek <andy@...yhouse.net>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH v2] net: bonding: Use strscpy() instead of manually-truncated strncpy() On Wed, Jun 02, 2021 at 01:47:20PM -0700, Jay Vosburgh wrote: > Kees Cook <keescook@...omium.org> wrote: > > >Silence this warning by just using strscpy_pad() directly: > > > >>> drivers/net/bonding/bond_main.c:4877:3: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation] > > 4877 | strncpy(params->primary, primary, IFNAMSIZ); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > >Additionally replace other strncpy() uses, as it is considered deprecated: > >https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > > > >Reported-by: kernel test robot <lkp@...el.com> > >Link: https://lore.kernel.org/lkml/202102150705.fdR6obB0-lkp@intel.com > >Signed-off-by: Kees Cook <keescook@...omium.org> > > There's one more "strncpy(...); primary[IFNAMSIZ - 1] = 0;" set > in bond_options.c:bond_option_primary_set(), doesn't it also generate > this warning? Gah, I was looking only in the same file not the whole directory. :) v3 on the way! > > Either way, the change looks good to me. Thanks! > > Acked-by: Jay Vosburgh <jay.vosburgh@...onical.com> > > -J > > > >--- > >v2: > > - switch to strscpy_pad() and replace earlier strncpy() too > >v1: https://lore.kernel.org/lkml/20210602181133.3326856-1-keescook@chromium.org > >--- > > drivers/net/bonding/bond_main.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > >index c5a646d06102..e9cb716ad849 100644 > >--- a/drivers/net/bonding/bond_main.c > >+++ b/drivers/net/bonding/bond_main.c > >@@ -620,7 +620,7 @@ static int bond_check_dev_link(struct bonding *bond, > > */ > > > > /* Yes, the mii is overlaid on the ifreq.ifr_ifru */ > >- strncpy(ifr.ifr_name, slave_dev->name, IFNAMSIZ); > >+ strscpy_pad(ifr.ifr_name, slave_dev->name, IFNAMSIZ); > > mii = if_mii(&ifr); > > if (ioctl(slave_dev, &ifr, SIOCGMIIPHY) == 0) { > > mii->reg_num = MII_BMSR; > >@@ -5329,10 +5329,8 @@ static int bond_check_params(struct bond_params *params) > > (struct reciprocal_value) { 0 }; > > } > > > >- if (primary) { > >- strncpy(params->primary, primary, IFNAMSIZ); > >- params->primary[IFNAMSIZ - 1] = 0; > >- } > >+ if (primary) > >+ strscpy_pad(params->primary, primary, sizeof(params->primary)); > > > > memcpy(params->arp_targets, arp_target, sizeof(arp_target)); > > > >-- > >2.25.1 > > --- > -Jay Vosburgh, jay.vosburgh@...onical.com -- Kees Cook
Powered by blists - more mailing lists