Re: [PATCH net-next] drivers/net/bonding: User strscpy() to copy device name
From: David Laight
Date: Wed Jun 10 2026 - 05:50:57 EST
On Tue, 9 Jun 2026 17:58:41 -0700
Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> On Sat, 6 Jun 2026 21:26:08 +0100 david.laight.linux@xxxxxxxxx wrote:
> > From: David Laight <david.laight.linux@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> > ---
> > 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;