Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[kernel test robot]

Hi Azeem,

kernel test robot noticed the following build errors:

[auto build test ERROR on uml/next]
[also build test ERROR on uml/fixes wireless-next/main wireless/main linus/master v6.4-rc4 next-20230530]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Azeem-Shaikh/uml-Replace-all-non-returning-strlcpy-with-strscpy/20230531-004115
base:   git://git.kernel.org/pub/scm/linux/kernel/git/uml/linux next
patch link:    https://lore.kernel.org/r/20230530164004.986750-1-azeemshaikh38%40gmail.com
patch subject: [PATCH] uml: Replace all non-returning strlcpy with strscpy
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230531/202305311135.zGMT1gYR-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/c51d7beb37cfbda321feb3811bbe0e381f804899
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Azeem-Shaikh/uml-Replace-all-non-returning-strlcpy-with-strscpy/20230531-004115
        git checkout c51d7beb37cfbda321feb3811bbe0e381f804899
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305311135.zGMT1gYR-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/um/os-Linux/drivers/tuntap_user.c: In function 'tuntap_open':
>> arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of function 'strscpy'; did you mean 'strncpy'? [-Werror=implicit-function-declaration]
     149 |                 strscpy(ifr.ifr_name, pri->dev_name, sizeof(ifr.ifr_name));
         |                 ^~~~~~~
         |                 strncpy
   cc1: some warnings being treated as errors


vim +149 arch/um/os-Linux/drivers/tuntap_user.c

   127	
   128	static int tuntap_open(void *data)
   129	{
   130		struct ifreq ifr;
   131		struct tuntap_data *pri = data;
   132		char *output, *buffer;
   133		int err, fds[2], len, used;
   134	
   135		err = tap_open_common(pri->dev, pri->gate_addr);
   136		if (err < 0)
   137			return err;
   138	
   139		if (pri->fixed_config) {
   140			pri->fd = os_open_file("/dev/net/tun",
   141					       of_cloexec(of_rdwr(OPENFLAGS())), 0);
   142			if (pri->fd < 0) {
   143				printk(UM_KERN_ERR "Failed to open /dev/net/tun, "
   144				       "err = %d\n", -pri->fd);
   145				return pri->fd;
   146			}
   147			memset(&ifr, 0, sizeof(ifr));
   148			ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
 > 149			strscpy(ifr.ifr_name, pri->dev_name, sizeof(ifr.ifr_name));
   150			if (ioctl(pri->fd, TUNSETIFF, &ifr) < 0) {
   151				err = -errno;
   152				printk(UM_KERN_ERR "TUNSETIFF failed, errno = %d\n",
   153				       errno);
   154				close(pri->fd);
   155				return err;
   156			}
   157		}
   158		else {
   159			err = socketpair(AF_UNIX, SOCK_DGRAM, 0, fds);
   160			if (err) {
   161				err = -errno;
   162				printk(UM_KERN_ERR "tuntap_open : socketpair failed - "
   163				       "errno = %d\n", errno);
   164				return err;
   165			}
   166	
   167			buffer = get_output_buffer(&len);
   168			if (buffer != NULL)
   169				len--;
   170			used = 0;
   171	
   172			err = tuntap_open_tramp(pri->gate_addr, &pri->fd, fds[0],
   173						fds[1], buffer, len, &used);
   174	
   175			output = buffer;
   176			if (err < 0) {
   177				printk("%s", output);
   178				free_output_buffer(buffer);
   179				printk(UM_KERN_ERR "tuntap_open_tramp failed - "
   180				       "err = %d\n", -err);
   181				return err;
   182			}
   183	
   184			pri->dev_name = uml_strdup(buffer);
   185			output += IFNAMSIZ;
   186			printk("%s", output);
   187			free_output_buffer(buffer);
   188	
   189			close(fds[0]);
   190			iter_addresses(pri->dev, open_addr, pri->dev_name);
   191		}
   192	
   193		return pri->fd;
   194	}
   195	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Kees Cook]

On May 30, 2023 8:18:42 PM PDT, kernel test robot <lkp@intel.com> wrote:
>Hi Azeem,
>
>kernel test robot noticed the following build errors:
>
>[auto build test ERROR on uml/next]
>[also build test ERROR on uml/fixes wireless-next/main wireless/main linus/master v6.4-rc4 next-20230530]
>[If your patch is applied to the wrong git tree, kindly drop us a note.
>And when submitting patch, we suggest to use '--base' as documented in
>https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
>url:    https://github.com/intel-lab-lkp/linux/commits/Azeem-Shaikh/uml-Replace-all-non-returning-strlcpy-with-strscpy/20230531-004115
>base:   git://git.kernel.org/pub/scm/linux/kernel/git/uml/linux next
>patch link:    https://lore.kernel.org/r/20230530164004.986750-1-azeemshaikh38%40gmail.com
>patch subject: [PATCH] uml: Replace all non-returning strlcpy with strscpy
>config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230531/202305311135.zGMT1gYR-lkp@intel.com/config)
>compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
>reproduce (this is a W=1 build):
>        # https://github.com/intel-lab-lkp/linux/commit/c51d7beb37cfbda321feb3811bbe0e381f804899
>        git remote add linux-review https://github.com/intel-lab-lkp/linux
>        git fetch --no-tags linux-review Azeem-Shaikh/uml-Replace-all-non-returning-strlcpy-with-strscpy/20230531-004115
>        git checkout c51d7beb37cfbda321feb3811bbe0e381f804899
>        # save the config file
>        mkdir build_dir && cp config build_dir/.config
>        make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
>        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash
>
>If you fix the issue, kindly add following tag where applicable
>| Reported-by: kernel test robot <lkp@intel.com>
>| Closes: https://lore.kernel.org/oe-kbuild-all/202305311135.zGMT1gYR-lkp@intel.com/
>
>All errors (new ones prefixed by >>):
>
>   arch/um/os-Linux/drivers/tuntap_user.c: In function 'tuntap_open':
>>> arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of function 'strscpy'; did you mean 'strncpy'? [-Werror=implicit-function-declaration]
>     149 |                 strscpy(ifr.ifr_name, pri->dev_name, sizeof(ifr.ifr_name));
>         |                 ^~~~~~~
>         |                 strncpy
>   cc1: some warnings being treated as errors

Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this strlcpy for now.

-Keed




-- 
Kees Cook

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Johannes Berg]

> -		strlcpy(ifr.ifr_name, pri->dev_name,
> sizeof(ifr.ifr_name));
> +		strscpy(ifr.ifr_name, pri->dev_name,
> sizeof(ifr.ifr_name));
> 

> >   arch/um/os-Linux/drivers/tuntap_user.c: In function 'tuntap_open':
> > > > arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of function 'strscpy'; did you mean 'strncpy'? [-Werror=implicit-function-declaration]
> >     149 |                 strscpy(ifr.ifr_name, pri->dev_name, sizeof(ifr.ifr_name));
> >         |                 ^~~~~~~
> >         |                 strncpy
> >   cc1: some warnings being treated as errors
> 
> Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this strlcpy for now.
> 

Well, strlcpy() isn't part of libc either, so all this would need is to
add it to user.h just like strlcpy() is now?

johannes

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Richard Weinberger]

----- Ursprüngliche Mail -----
>> Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this
>> strlcpy for now.
>> 
> 
> Well, strlcpy() isn't part of libc either, so all this would need is to
> add it to user.h just like strlcpy() is now?

I think so.
Azeem, can you please test your changes with this fixup applied?

diff --git a/arch/um/include/shared/user.h b/arch/um/include/shared/user.h
index bda66e5a9d4e3..e5d3fbbafe4d2 100644
--- a/arch/um/include/shared/user.h
+++ b/arch/um/include/shared/user.h
@@ -52,6 +52,7 @@ static inline int printk(const char *fmt, ...)
 extern int in_aton(char *str);
 extern size_t strlcpy(char *, const char *, size_t);
 extern size_t strlcat(char *, const char *, size_t);
+extern ssize_t strscpy(char *, const char *, size_t);
 
 /* Copied from linux/compiler-gcc.h since we can't include it directly */
 #define barrier() __asm__ __volatile__("": : :"memory")

Thanks,
//richard

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Richard Weinberger]

----- Ursprüngliche Mail -----
>>>> arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of
>>>> function 'strscpy'; did you mean 'strncpy'?
>>>> [-Werror=implicit-function-declaration]
>>     149 |                 strscpy(ifr.ifr_name, pri->dev_name,
>>     sizeof(ifr.ifr_name));
>>         |                 ^~~~~~~
>>         |                 strncpy
>>   cc1: some warnings being treated as errors
> 
> Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this
> strlcpy for now.

Well, actually it's another case of "not even compile tested". :-(

Thanks,
//richard

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Geert Uytterhoeven]

On Wed, May 31, 2023 at 8:23 AM Richard Weinberger <richard@nod.at> wrote:
> ----- Ursprüngliche Mail -----
> >>>> arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of
> >>>> function 'strscpy'; did you mean 'strncpy'?
> >>>> [-Werror=implicit-function-declaration]
> >>     149 |                 strscpy(ifr.ifr_name, pri->dev_name,
> >>     sizeof(ifr.ifr_name));
> >>         |                 ^~~~~~~
> >>         |                 strncpy
> >>   cc1: some warnings being treated as errors
> >
> > Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this
> > strlcpy for now.
>
> Well, actually it's another case of "not even compile tested". :-(

"But the AI said it was correct?!?" ;-)

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] uml: Replace all non-returning strlcpy with strscpy

[Azeem Shaikh]

Thanks Geert and Richard for the review.

On Wed, May 31, 2023 at 4:05 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Wed, May 31, 2023 at 8:23 AM Richard Weinberger <richard@nod.at> wrote:
> > ----- Ursprüngliche Mail -----
> > >>>> arch/um/os-Linux/drivers/tuntap_user.c:149:17: error: implicit declaration of
> > >>>> function 'strscpy'; did you mean 'strncpy'?
> > >>>> [-Werror=implicit-function-declaration]
> > >>     149 |                 strscpy(ifr.ifr_name, pri->dev_name,
> > >>     sizeof(ifr.ifr_name));
> > >>         |                 ^~~~~~~
> > >>         |                 strncpy
> > >>   cc1: some warnings being treated as errors
> > >
> > > Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this
> > > strlcpy for now.
> >
> > Well, actually it's another case of "not even compile tested". :-(

Argh, my test script wasn't cross-compiling for um. Sorry about that :(

> > Well, strlcpy() isn't part of libc either, so all this would need is to
> > add it to user.h just like strlcpy() is now?
>
> I think so.
> Azeem, can you please test your changes with this fixup applied?
>
> diff --git a/arch/um/include/shared/user.h b/arch/um/include/shared/user.h
> index bda66e5a9d4e3..e5d3fbbafe4d2 100644
> --- a/arch/um/include/shared/user.h
> +++ b/arch/um/include/shared/user.h
> @@ -52,6 +52,7 @@ static inline int printk(const char *fmt, ...)
>  extern int in_aton(char *str);
>  extern size_t strlcpy(char *, const char *, size_t);
>  extern size_t strlcat(char *, const char *, size_t);
> +extern ssize_t strscpy(char *, const char *, size_t);
>
>  /* Copied from linux/compiler-gcc.h since we can't include it directly */
>  #define barrier() __asm__ __volatile__("": : :"memory")
>

Tested with this fixup, and it builds successfully on ARCH=um
SUBARCH=i386. Let me know if I need to test with any other
cross-compilation options before I send out v2.

>
> Ah, yeah, this is another "not actually in the kernel" cases. Let's ignore this strlcpy for now.
>
> -Keed

Planning to send out v2 with the fixup from Richard applied. Let me
know if that's ok.

Re: [PATCH] uml: Replace all non-returning strlcpy with strscpy

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Azeem Shaikh" <azeemshaikh38@gmail.com>
> Planning to send out v2 with the fixup from Richard applied. Let me
> know if that's ok.

Fine by me. :-)

Thanks,
//richard