linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
@ 2020-05-22 13:58 kbuild test robot
  2020-05-22 14:23 ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: kbuild test robot @ 2020-05-22 13:58 UTC (permalink / raw)
  To: Al Viro; +Cc: kbuild-all, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 5211 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git uaccess.net
head:   0edecc020b33f8e31d8baa80735b45e8e8434700
commit: a3929484af75ee524419edbbc4e9ce012c3d67c9 [16/19] atm: move copyin from atm_getnames() into the caller
config: arm64-randconfig-s002-20200521 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-193-gb8fad4bc-dirty
        git checkout a3929484af75ee524419edbbc4e9ce012c3d67c9
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=arm64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer

vim +180 net/atm/ioctl.c

    50	
    51	static int do_vcc_ioctl(struct socket *sock, unsigned int cmd,
    52				unsigned long arg, int compat)
    53	{
    54		struct sock *sk = sock->sk;
    55		struct atm_vcc *vcc;
    56		int error;
    57		struct list_head *pos;
    58		void __user *argp = (void __user *)arg;
    59		void __user *buf;
    60		int __user *len;
    61	
    62		vcc = ATM_SD(sock);
    63		switch (cmd) {
    64		case SIOCOUTQ:
    65			if (sock->state != SS_CONNECTED ||
    66			    !test_bit(ATM_VF_READY, &vcc->flags)) {
    67				error =  -EINVAL;
    68				goto done;
    69			}
    70			error = put_user(sk->sk_sndbuf - sk_wmem_alloc_get(sk),
    71					 (int __user *)argp) ? -EFAULT : 0;
    72			goto done;
    73		case SIOCINQ:
    74		{
    75			struct sk_buff *skb;
    76	
    77			if (sock->state != SS_CONNECTED) {
    78				error = -EINVAL;
    79				goto done;
    80			}
    81			skb = skb_peek(&sk->sk_receive_queue);
    82			error = put_user(skb ? skb->len : 0,
    83					 (int __user *)argp) ? -EFAULT : 0;
    84			goto done;
    85		}
    86		case ATM_SETSC:
    87			net_warn_ratelimited("ATM_SETSC is obsolete; used by %s:%d\n",
    88					     current->comm, task_pid_nr(current));
    89			error = 0;
    90			goto done;
    91		case ATMSIGD_CTRL:
    92			if (!capable(CAP_NET_ADMIN)) {
    93				error = -EPERM;
    94				goto done;
    95			}
    96			/*
    97			 * The user/kernel protocol for exchanging signalling
    98			 * info uses kernel pointers as opaque references,
    99			 * so the holder of the file descriptor can scribble
   100			 * on the kernel... so we should make sure that we
   101			 * have the same privileges that /proc/kcore needs
   102			 */
   103			if (!capable(CAP_SYS_RAWIO)) {
   104				error = -EPERM;
   105				goto done;
   106			}
   107	#ifdef CONFIG_COMPAT
   108			/* WTF? I don't even want to _think_ about making this
   109			   work for 32-bit userspace. TBH I don't really want
   110			   to think about it at all. dwmw2. */
   111			if (compat) {
   112				net_warn_ratelimited("32-bit task cannot be atmsigd\n");
   113				error = -EINVAL;
   114				goto done;
   115			}
   116	#endif
   117			error = sigd_attach(vcc);
   118			if (!error)
   119				sock->state = SS_CONNECTED;
   120			goto done;
   121		case ATM_SETBACKEND:
   122		case ATM_NEWBACKENDIF:
   123		{
   124			atm_backend_t backend;
   125			error = get_user(backend, (atm_backend_t __user *)argp);
   126			if (error)
   127				goto done;
   128			switch (backend) {
   129			case ATM_BACKEND_PPP:
   130				request_module("pppoatm");
   131				break;
   132			case ATM_BACKEND_BR2684:
   133				request_module("br2684");
   134				break;
   135			}
   136			break;
   137		}
   138		case ATMMPC_CTRL:
   139		case ATMMPC_DATA:
   140			request_module("mpoa");
   141			break;
   142		case ATMARPD_CTRL:
   143			request_module("clip");
   144			break;
   145		case ATMLEC_CTRL:
   146			request_module("lec");
   147			break;
   148		}
   149	
   150		error = -ENOIOCTLCMD;
   151	
   152		mutex_lock(&ioctl_mutex);
   153		list_for_each(pos, &ioctl_list) {
   154			struct atm_ioctl *ic = list_entry(pos, struct atm_ioctl, list);
   155			if (try_module_get(ic->owner)) {
   156				error = ic->ioctl(sock, cmd, arg);
   157				module_put(ic->owner);
   158				if (error != -ENOIOCTLCMD)
   159					break;
   160			}
   161		}
   162		mutex_unlock(&ioctl_mutex);
   163	
   164		if (error != -ENOIOCTLCMD)
   165			goto done;
   166	
   167		if (cmd == ATM_GETNAMES) {
   168			if (IS_ENABLED(CONFIG_COMPAT) && compat) {
   169	#ifdef CONFIG_COMPAT
   170				struct compat_atm_iobuf __user *ciobuf = argp;
   171				compat_uptr_t cbuf;
   172				len = &ciobuf->length;
   173				if (get_user(cbuf, &ciobuf->buffer))
   174					return -EFAULT;
   175				buf = compat_ptr(cbuf);
   176	#endif
   177			} else {
   178				struct atm_iobuf __user *iobuf = argp;
   179				len = &iobuf->length;
 > 180				if (get_user(buf, &iobuf->buffer))
   181					return -EFAULT;
   182			}
   183			error = atm_getnames(buf, len);
   184		} else {
   185			error = atm_dev_ioctl(cmd, argp, compat);
   186		}
   187	
   188	done:
   189		return error;
   190	}
   191	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41626 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
  2020-05-22 13:58 [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer kbuild test robot
@ 2020-05-22 14:23 ` Al Viro
  2020-05-22 14:44   ` Catalin Marinas
  0 siblings, 1 reply; 4+ messages in thread
From: Al Viro @ 2020-05-22 14:23 UTC (permalink / raw)
  To: kbuild test robot; +Cc: kbuild-all, linux-fsdevel, Catalin Marinas

On Fri, May 22, 2020 at 09:58:00PM +0800, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git uaccess.net
> head:   0edecc020b33f8e31d8baa80735b45e8e8434700
> commit: a3929484af75ee524419edbbc4e9ce012c3d67c9 [16/19] atm: move copyin from atm_getnames() into the caller
> config: arm64-randconfig-s002-20200521 (attached as .config)
> compiler: aarch64-linux-gcc (GCC) 9.3.0
> reproduce:
>         # apt-get install sparse
>         # sparse version: v0.6.1-193-gb8fad4bc-dirty
>         git checkout a3929484af75ee524419edbbc4e9ce012c3d67c9
>         # save the attached .config to linux build tree
>         make W=1 C=1 ARCH=arm64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
> >> net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer

Huh?

>  > 180				if (get_user(buf, &iobuf->buffer))

_what_ use of plain integer as a NULL pointer?  <looks>
Misannotated get_user() on arm64 - should be
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 32fc8061aa76..bc5c7b091152 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -304,7 +304,7 @@ do {									\
 		__p = uaccess_mask_ptr(__p);				\
 		__raw_get_user((x), __p, (err));			\
 	} else {							\
-		(x) = 0; (err) = -EFAULT;				\
+		(x) = (__force __typeof__(x))0; (err) = -EFAULT;	\
 	}								\
 } while (0)
 
and that's a _lot_ of noise in sparse logs on arm64, obviously - one for
each get_user() of a pointer...

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
  2020-05-22 14:23 ` Al Viro
@ 2020-05-22 14:44   ` Catalin Marinas
  2020-05-22 15:04     ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: Catalin Marinas @ 2020-05-22 14:44 UTC (permalink / raw)
  To: Al Viro; +Cc: kbuild test robot, kbuild-all, linux-fsdevel

On Fri, May 22, 2020 at 03:23:21PM +0100, Al Viro wrote:
> On Fri, May 22, 2020 at 09:58:00PM +0800, kbuild test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git uaccess.net
> > head:   0edecc020b33f8e31d8baa80735b45e8e8434700
> > commit: a3929484af75ee524419edbbc4e9ce012c3d67c9 [16/19] atm: move copyin from atm_getnames() into the caller
> > config: arm64-randconfig-s002-20200521 (attached as .config)
> > compiler: aarch64-linux-gcc (GCC) 9.3.0
> > reproduce:
> >         # apt-get install sparse
> >         # sparse version: v0.6.1-193-gb8fad4bc-dirty
> >         git checkout a3929484af75ee524419edbbc4e9ce012c3d67c9
> >         # save the attached .config to linux build tree
> >         make W=1 C=1 ARCH=arm64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kbuild test robot <lkp@intel.com>
> > 
> > 
> > sparse warnings: (new ones prefixed by >>)
> > 
> > >> net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
> 
> Huh?
> 
> >  > 180				if (get_user(buf, &iobuf->buffer))
> 
> _what_ use of plain integer as a NULL pointer?  <looks>
> Misannotated get_user() on arm64 - should be
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 32fc8061aa76..bc5c7b091152 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -304,7 +304,7 @@ do {									\
>  		__p = uaccess_mask_ptr(__p);				\
>  		__raw_get_user((x), __p, (err));			\
>  	} else {							\
> -		(x) = 0; (err) = -EFAULT;				\
> +		(x) = (__force __typeof__(x))0; (err) = -EFAULT;	\
>  	}								\
>  } while (0)
>  
> and that's a _lot_ of noise in sparse logs on arm64, obviously - one for
> each get_user() of a pointer...

Thanks for pointing this out. We seem to do the right thing for
__raw_get_user() but missed one path in __get_user_error().

Since you wrote the patch already, are you ok for me to add some text
and your signed-off-by?

-- 
Catalin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
  2020-05-22 14:44   ` Catalin Marinas
@ 2020-05-22 15:04     ` Al Viro
  0 siblings, 0 replies; 4+ messages in thread
From: Al Viro @ 2020-05-22 15:04 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: kbuild test robot, kbuild-all, linux-fsdevel

On Fri, May 22, 2020 at 03:44:40PM +0100, Catalin Marinas wrote:
> On Fri, May 22, 2020 at 03:23:21PM +0100, Al Viro wrote:
> > On Fri, May 22, 2020 at 09:58:00PM +0800, kbuild test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git uaccess.net
> > > head:   0edecc020b33f8e31d8baa80735b45e8e8434700
> > > commit: a3929484af75ee524419edbbc4e9ce012c3d67c9 [16/19] atm: move copyin from atm_getnames() into the caller
> > > config: arm64-randconfig-s002-20200521 (attached as .config)
> > > compiler: aarch64-linux-gcc (GCC) 9.3.0
> > > reproduce:
> > >         # apt-get install sparse
> > >         # sparse version: v0.6.1-193-gb8fad4bc-dirty
> > >         git checkout a3929484af75ee524419edbbc4e9ce012c3d67c9
> > >         # save the attached .config to linux build tree
> > >         make W=1 C=1 ARCH=arm64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> > > 
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > > 
> > > 
> > > sparse warnings: (new ones prefixed by >>)
> > > 
> > > >> net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer
> > 
> > Huh?
> > 
> > >  > 180				if (get_user(buf, &iobuf->buffer))
> > 
> > _what_ use of plain integer as a NULL pointer?  <looks>
> > Misannotated get_user() on arm64 - should be
> > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> > index 32fc8061aa76..bc5c7b091152 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -304,7 +304,7 @@ do {									\
> >  		__p = uaccess_mask_ptr(__p);				\
> >  		__raw_get_user((x), __p, (err));			\
> >  	} else {							\
> > -		(x) = 0; (err) = -EFAULT;				\
> > +		(x) = (__force __typeof__(x))0; (err) = -EFAULT;	\
> >  	}								\
> >  } while (0)
> >  
> > and that's a _lot_ of noise in sparse logs on arm64, obviously - one for
> > each get_user() of a pointer...
> 
> Thanks for pointing this out. We seem to do the right thing for
> __raw_get_user() but missed one path in __get_user_error().
> 
> Since you wrote the patch already, are you ok for me to add some text
> and your signed-off-by?

Sure, np...

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-22 15:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 13:58 [vfs:uaccess.net 16/19] net/atm/ioctl.c:180:29: sparse: sparse: Using plain integer as NULL pointer kbuild test robot
2020-05-22 14:23 ` Al Viro
2020-05-22 14:44   ` Catalin Marinas
2020-05-22 15:04     ` Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).