Re: [PATCH] sh: use generic strncpy()

Re: [PATCH] sh: use generic strncpy()

[Geert Uytterhoeven]

Hi Morimoto-san, Karl,

On Wed, Dec 18, 2019 at 6:22 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Current SH will get below warning at strncpy()
>
> In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
>                  from ${LINUX}/include/linux/string.h:20,
>                  from ${LINUX}/include/linux/bitmap.h:9,
>                  from ${LINUX}/include/linux/nodemask.h:95,
>                  from ${LINUX}/include/linux/mmzone.h:17,
>                  from ${LINUX}/include/linux/gfp.h:6,
>                  from ${LINUX}/innclude/linux/slab.h:15,
>                  from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
> ${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> ${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
>   80 is above array bounds of 'char[26]' [-Warray-bounds]
>    : "0" (__dest), "1" (__src), "r" (__src+__n)
>                                      ~~~~~^~~~
>
> In general, strncpy() should behave like below.
>
>         char dest[10];
>         char *src = "12345";
>
>         strncpy(dest, src, 10);
>         // dest = {'1', '2', '3', '4', '5',
>                    '\0','\0','\0','\0','\0'}
>
> But, current SH strnpy() has 2 issues.
> 1st is it will access to out-of-memory (= src + 10).

I believe this is not correct: the code does not really access memory
beyond the end of the source string.  (Recent) gcc just thinks so,
because "__src+__n" is used as a parameter to the routine.

> 2nd is it needs big fixup for it, and maintenance __asm__
> code is difficult.

Yeah, the padding is missing.

> To solve these issues, this patch simply uses generic strncpy()
> instead of architecture specific one.

That will definitely fix the issue, as we assume the generic
implementation is correct ;-)

Now, I've just tried, naively, to enable CONFIG_STRING_SELFTEST=y in my
rts7751r2d build (without your patch), and boot it in qemu:

    String selftests succeeded

Woops, turns out lib/test_string.c does not have any testcases for
strncpy()...

So adding test code for the corner cases may be a valuable contribution.
Thanks!

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: use generic strncpy()

[Kuninori Morimoto]

Hi Geert

Thank you for your review

> > But, current SH strnpy() has 2 issues.
> > 1st is it will access to out-of-memory (= src + 10).
> 
> I believe this is not correct: the code does not really access memory
> beyond the end of the source string.  (Recent) gcc just thinks so,
> because "__src+__n" is used as a parameter to the routine.

I see.
Will fix in v2

Thank you for your help !!
Best regards
---
Kuninori Morimoto