Messages in this thread | | | From | Jay Vosburgh <> | Subject | Re: [PATCH v2] net: bonding: Use strscpy() instead of manually-truncated strncpy() | Date | Wed, 02 Jun 2021 13:47:20 -0700 |
| |
Kees Cook <keescook@chromium.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@intel.com> >Link: https://lore.kernel.org/lkml/202102150705.fdR6obB0-lkp@intel.com >Signed-off-by: Kees Cook <keescook@chromium.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?
Either way, the change looks good to me.
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.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@canonical.com
| |