Re: [PATCH] Fix possible strscpy() buffer overflows

[David Laight]

On Mon, 11 May 2026 14:51:37 +0200
Borislav Petkov <bp@xxxxxxxxx> wrote:

> On Mon, May 11, 2026 at 11:59:34AM +0000, Andrei Purdea wrote:
> > No, the current code copies "error_ip" with null termination, and it
> > drops the "c" suffix.  
> 
> Pfff, that wasn't really clear to me from the explanation of strscpy...
> 
> > And that seems buggy. And that's what I requested to explain the effects of.  

It is a new(ish) driver.
It looks like just a bug.
Although the name itself looks strange.

> 
> Shubhrajyoti, does that have any visible effects when using the driver?
> 
> > strscpy_pad(chinfo.name, amd_rpmsg_id_table[0].name);  
> 
> No, as said "[h]owever, just to make this safer, we should min the size".
> 
> IOW:
> 
>          strscpy_pad(chinfo.name,
>                      amd_rpmsg_id_table[0].name,
>                      min_t(size_t, strlen(amd_rpmsg_id_table[0].name) + 1, RPMSG_NAME_SIZE));

That is just pure crap.
	strscpy(chinfo.name, amd_rpmsg_id_table[0].name);
will DTRT pretty much regardless of any obscurities - including the case where the
source array isn't '\0' terminated (although you might get a nasty run-time
error message if it is shorter than the destination).

Note that the inlined 'mess' generated for strscpy() in fortify_string.h isn't
ideal for all sorts of reasons.
(Not least because the 'hard' cases should be out of line.)
But that is an unrelated issue.

It would be better if amd_rpmsg_id_table[] were 'const' - in that case the strscpy()
call should 'degenerate' into a memcpy().

-- David


> 
> In case someone goes and changes that amd_rpmsg_id_table[0].name in the
> future.
>