Re: [PATCH next] fs: Replace strcpy(s, "../") with memcpy(s, "../", 4)

[David Laight]

On Fri, 19 Jun 2026 12:58:20 +0200
Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:

> <david.laight.linux@xxxxxxxxx> writes:
> 
> > From: David Laight <david.laight.linux@xxxxxxxxx>
> >
> > The code has already checked there is enough room.
> > Use memcpy() to avoid compiler warnings from possibly unbounded strcpy().
> >
> > 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.)
> >
> >  fs/configfs/symlink.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
> > index f3f79c67add5..9f36699e5922 100644
> > --- a/fs/configfs/symlink.c
> > +++ b/fs/configfs/symlink.c
> > @@ -67,7 +67,7 @@ static int configfs_get_target_path(struct config_item *item,
> >  	pr_debug("%s: depth = %d, size = %d\n", __func__, depth, size);
> >
> >  	for (s = path; depth--; s += 3)
> > -		strcpy(s,"../");
> > +		memcpy(s, "../", 4);  
> 
> I don't think this transform makes sense when copying string literals.
> The post transform code has one more foot gun than the original code.

They are actually identical, the compiler converts the former to the latter.

I was trying to remove all the strcpy() where the target isn't an array.
The initial check also only allowed string literals - but I relaxed that
a bit to reduce the number of false positives.
Were a similar check for calls to strcpy() be committed this code would
need changing, but you want something that ends up being a (misaligned)
32bit write of a constant on most architectures.  

I just looked at the code again, the final '- 1' on the 'size = ...'
line looks very odd.

I wonder if it would be simpler to merge all three functions into
something with a single loop that builds the name/name part backwards
from the end of the buffer while adding "../" on the front and then
calling memmove() to put the two together.

	David

> 
> Best regards,
> Andreas Hindborg
> 
> 
>