Re: [PATCH net-next] drivers/net/bonding: User strscpy() to copy device name

[David Laight <david.laight.linux@gmail.com>]

On Tue, 9 Jun 2026 17:58:41 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Sat,  6 Jun 2026 21:26:08 +0100 david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>  
> 
> Commit message is required. Please explain why you think this patch is
> needed. AFAICT it copies data between two well formed IFNAMSIZ strings.

Thinks...
I let strcpy(xx->array, "constant") through provided the array is big enough.
This gets converted to memcpy().
I could also check the array sizes for strcpy(xx->array, yy->array) and
allow provided the destination isn't shorter.
That would remove some of the 'annoying false positives'.
Unlike strscpy() this could be converted to a memcpy() (with or without
explicitly writing the terminating '\0') for short (say <= 32 byte)
lengths.

So this patch (and a few like it) can be dropped.
Some one else may want to remove strcpy() completely, but I was only
trying to remove the ones that a simple compile-time test couldn't
show were safe.

-- David

> 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > This is one of a group of patches that remove potentially unbounded
> > strcpy() calls.
> > 
> > They are mostly replaced by strscpy() or, when strlen() has just been
> > called, with memcpy() (usually including the '\0').
> > 
> > Calls with copy string literals into arrays are left unchanged.
> > They are safe and easily detected as such.
> > 
> > The changes were made by getting the compiler to detect the calls and
> > then fixing the code by hand.
> > 
> > Note that all the changes are only compile tested.
> > 
> > Some Makefiles were changed to allow files to contain strcpy().
> > As well as 'difficult to fix' files, this included 'show' functions
> > as they really need to use sysfs_emit() or seq_printf().
> > 
> > All the patches are being sent individually to avoid very long cc lists.
> > Apologies for the terse commit messages and likely unexpected tags.
> > (There are about 100 patches in total.)
> > 
> >  drivers/net/bonding/bond_options.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> > index 7380cc4ee75a..c57b7d6af043 100644
> > --- a/drivers/net/bonding/bond_options.c
> > +++ b/drivers/net/bonding/bond_options.c
> > @@ -1525,7 +1525,7 @@ static int bond_option_primary_set(struct bonding *bond,
> >  		if (strncmp(slave->dev->name, primary, IFNAMSIZ) == 0) {
> >  			slave_dbg(bond->dev, slave->dev, "Setting as primary slave\n");
> >  			rcu_assign_pointer(bond->primary_slave, slave);
> > -			strcpy(bond->params.primary, slave->dev->name);
> > +			strscpy(bond->params.primary, slave->dev->name);
> >  			bond->force_primary = true;
> >  			bond_select_active_slave(bond);
> >  			goto out;