From: Geert Uytterhoeven <geert@linux-m68k.org> To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>, Karl Nasrallah <knnspeed@aol.com> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>, Rich Felker <dalias@libc.org>, Linux-SH <linux-sh@vger.kernel.org>, Linux-Renesas <linux-renesas-soc@vger.kernel.org>, "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org> Subject: Re: [PATCH] sh: use generic strncpy() Date: Wed, 18 Dec 2019 08:02:00 +0000 [thread overview] Message-ID: <CAMuHMdUq6U0i_+Dg57jVNYM4iephuZ8k3QC6AyQ--W_qY5=q9w@mail.gmail.com> (raw) In-Reply-To: <878snajjh2.wl-kuninori.morimoto.gx@renesas.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Geert Uytterhoeven <geert@linux-m68k.org> To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>, Karl Nasrallah <knnspeed@aol.com> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>, Rich Felker <dalias@libc.org>, Linux-SH <linux-sh@vger.kernel.org>, Linux-Renesas <linux-renesas-soc@vger.kernel.org>, "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org> Subject: Re: [PATCH] sh: use generic strncpy() Date: Wed, 18 Dec 2019 09:02:00 +0100 [thread overview] Message-ID: <CAMuHMdUq6U0i_+Dg57jVNYM4iephuZ8k3QC6AyQ--W_qY5=q9w@mail.gmail.com> (raw) In-Reply-To: <878snajjh2.wl-kuninori.morimoto.gx@renesas.com> 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
next prev parent reply other threads:[~2019-12-18 8:02 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-18 5:22 [PATCH] sh: use generic strncpy() Kuninori Morimoto 2019-12-18 5:22 ` Kuninori Morimoto 2019-12-18 8:02 ` Geert Uytterhoeven [this message] 2019-12-18 8:02 ` Geert Uytterhoeven 2019-12-19 0:45 ` Kuninori Morimoto 2019-12-19 0:45 ` Kuninori Morimoto 2019-12-18 7:31 [PATCH 0/2] " Kuninori Morimoto 2019-12-18 7:32 ` [PATCH] " Kuninori Morimoto 2019-12-18 7:32 ` Kuninori Morimoto 2019-12-18 7:35 ` Kuninori Morimoto 2019-12-18 7:35 ` Kuninori Morimoto
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAMuHMdUq6U0i_+Dg57jVNYM4iephuZ8k3QC6AyQ--W_qY5=q9w@mail.gmail.com' \ --to=geert@linux-m68k.org \ --cc=dalias@libc.org \ --cc=knnspeed@aol.com \ --cc=kuninori.morimoto.gx@renesas.com \ --cc=linux-kselftest@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=linux-sh@vger.kernel.org \ --cc=ysato@users.sourceforge.jp \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.