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

[Borislav Petkov]

On Mon, May 11, 2026 at 09:46:53AM +0300, Andrei Purdea wrote:
> Hi all,

Hi,

first of all, please do not top-post when replying to a public mailing list.

> Furthermore the one in versalnet_edac.c looks like beyond fixing a
> buffer overflow risk and code smell, it also introduces a behavior
> change (bugfix?), because the old code I believe cuts off the last
> character from the copied string. (Because it was using just strlen()
> and not using strlen() + 1)

The versalnet_edac is innocuous because, AFAICT, that silly code is copying
"error_ipc" into

struct rpmsg_channel_info {
        char name[RPMSG_NAME_SIZE];

with that buffer size being 32.

So no overflow here.

However, just to make this safer, we should min the size.

Also, Shubhrajyoti, why aren't you padding the rest of the name string with \0
*especially* since you're allocating it on the stack?!

IOW, this (totally untested ofc):

        strscpy_pad(chinfo.name,
                    amd_rpmsg_id_table[0].name,
                    min_t(size_t, strlen(amd_rpmsg_id_table[0].name), RPMSG_NAME_SIZE));

I still don't like it because we're noodling with this string as if it is
super special and copying it back'n'forth just because this rpmsg is
MODULE_DEVICE_TABLE and apparently is used to autoload the driver or so...

Why do we need it?

Can we simplify that gunk there?

Please have a look.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette