linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
@ 2025-11-27 19:29 kernel test robot
  2025-11-29 19:51 ` Pasha Tatashin
  0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2025-11-27 19:29 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: oe-kbuild-all, Andrew Morton, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   ef68bf704646690aba5e81c2f7be8d6ef13d7ad8
commit: b30a1fee674c8446bebd9e399b267ac824093bc8 [12118/12398] liveupdate: luo_core: add user interface
config: x86_64-randconfig-161-20251127 (https://download.01.org/0day-ci/archive/20251128/202511280300.6pvBmXUS-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511280300.6pvBmXUS-lkp@intel.com/

smatch warnings:
kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.

vim +/nr +402 kernel/liveupdate/luo_core.c

   392	
   393	static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
   394	{
   395		const struct luo_ioctl_op *op;
   396		struct luo_ucmd ucmd = {};
   397		union ucmd_buffer buf;
   398		unsigned int nr;
   399		int err;
   400	
   401		nr = _IOC_NR(cmd);
 > 402		if (nr < LIVEUPDATE_CMD_BASE ||
   403		    (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {
   404			return -EINVAL;
   405		}
   406	
   407		ucmd.ubuffer = (void __user *)arg;
   408		err = get_user(ucmd.user_size, (u32 __user *)ucmd.ubuffer);
   409		if (err)
   410			return err;
   411	
   412		op = &luo_ioctl_ops[nr - LIVEUPDATE_CMD_BASE];
   413		if (op->ioctl_num != cmd)
   414			return -ENOIOCTLCMD;
   415		if (ucmd.user_size < op->min_size)
   416			return -EINVAL;
   417	
   418		ucmd.cmd = &buf;
   419		err = copy_struct_from_user(ucmd.cmd, op->size, ucmd.ubuffer,
   420					    ucmd.user_size);
   421		if (err)
   422			return err;
   423	
   424		return op->execute(&ucmd);
   425	}
   426	

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


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

* Re: [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
  2025-11-27 19:29 [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero kernel test robot
@ 2025-11-29 19:51 ` Pasha Tatashin
  2025-11-29 20:00   ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Pasha Tatashin @ 2025-11-29 19:51 UTC (permalink / raw)
  To: kernel test robot
  Cc: oe-kbuild-all, Andrew Morton, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

On Thu, Nov 27, 2025 at 2:29 PM kernel test robot <lkp@intel.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   ef68bf704646690aba5e81c2f7be8d6ef13d7ad8
> commit: b30a1fee674c8446bebd9e399b267ac824093bc8 [12118/12398] liveupdate: luo_core: add user interface
> config: x86_64-randconfig-161-20251127 (https://download.01.org/0day-ci/archive/20251128/202511280300.6pvBmXUS-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202511280300.6pvBmXUS-lkp@intel.com/
>
> smatch warnings:
> kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
>
> vim +/nr +402 kernel/liveupdate/luo_core.c
>
>    392
>    393  static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>    394  {
>    395          const struct luo_ioctl_op *op;
>    396          struct luo_ucmd ucmd = {};
>    397          union ucmd_buffer buf;
>    398          unsigned int nr;
>    399          int err;
>    400
>    401          nr = _IOC_NR(cmd);
>  > 402          if (nr < LIVEUPDATE_CMD_BASE ||
>    403              (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {

This is a false positive. The logic is designed to work generically
for any base, similar to how it is handled in session ioctl and
iommufd ictl. The warning only triggers because LIVEUPDATE_CMD_BASE
happens to be 0 in this specific case.

>    404                  return -EINVAL;
>    405          }
>    406
>    407          ucmd.ubuffer = (void __user *)arg;
>    408          err = get_user(ucmd.user_size, (u32 __user *)ucmd.ubuffer);
>    409          if (err)
>    410                  return err;
>    411
>    412          op = &luo_ioctl_ops[nr - LIVEUPDATE_CMD_BASE];
>    413          if (op->ioctl_num != cmd)
>    414                  return -ENOIOCTLCMD;
>    415          if (ucmd.user_size < op->min_size)
>    416                  return -EINVAL;
>    417
>    418          ucmd.cmd = &buf;
>    419          err = copy_struct_from_user(ucmd.cmd, op->size, ucmd.ubuffer,
>    420                                      ucmd.user_size);
>    421          if (err)
>    422                  return err;
>    423
>    424          return op->execute(&ucmd);
>    425  }
>    426
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki


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

* Re: [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
  2025-11-29 19:51 ` Pasha Tatashin
@ 2025-11-29 20:00   ` Andrew Morton
  2025-11-29 20:20     ` Pasha Tatashin
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2025-11-29 20:00 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

On Sat, 29 Nov 2025 14:51:28 -0500 Pasha Tatashin <pasha.tatashin@soleen.com> wrote:

> > smatch warnings:
> > kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
> >
> > vim +/nr +402 kernel/liveupdate/luo_core.c
> >
> >    392
> >    393  static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> >    394  {
> >    395          const struct luo_ioctl_op *op;
> >    396          struct luo_ucmd ucmd = {};
> >    397          union ucmd_buffer buf;
> >    398          unsigned int nr;
> >    399          int err;
> >    400
> >    401          nr = _IOC_NR(cmd);
> >  > 402          if (nr < LIVEUPDATE_CMD_BASE ||
> >    403              (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {
> 
> This is a false positive. The logic is designed to work generically
> for any base, similar to how it is handled in session ioctl and
> iommufd ictl. The warning only triggers because LIVEUPDATE_CMD_BASE
> happens to be 0 in this specific case.

This is a difficult situation.

In some cases smatch has found a bug, in other cases it's just being
irritating.

I think there is value in smatch occasionally finding bugs, and to
support that we should try to rearrange false positives so that smatch
doesn't report on those.

Perhaps there's some way we can do that here, without uglifying things?


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

* Re: [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
  2025-11-29 20:00   ` Andrew Morton
@ 2025-11-29 20:20     ` Pasha Tatashin
  2025-11-29 23:52       ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Pasha Tatashin @ 2025-11-29 20:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

On Sat, Nov 29, 2025 at 3:00 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sat, 29 Nov 2025 14:51:28 -0500 Pasha Tatashin <pasha.tatashin@soleen.com> wrote:
>
> > > smatch warnings:
> > > kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
> > >
> > > vim +/nr +402 kernel/liveupdate/luo_core.c
> > >
> > >    392
> > >    393  static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> > >    394  {
> > >    395          const struct luo_ioctl_op *op;
> > >    396          struct luo_ucmd ucmd = {};
> > >    397          union ucmd_buffer buf;
> > >    398          unsigned int nr;
> > >    399          int err;
> > >    400
> > >    401          nr = _IOC_NR(cmd);
> > >  > 402          if (nr < LIVEUPDATE_CMD_BASE ||
> > >    403              (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {
> >
> > This is a false positive. The logic is designed to work generically
> > for any base, similar to how it is handled in session ioctl and
> > iommufd ictl. The warning only triggers because LIVEUPDATE_CMD_BASE
> > happens to be 0 in this specific case.
>
> This is a difficult situation.
>
> In some cases smatch has found a bug, in other cases it's just being
> irritating.
>
> I think there is value in smatch occasionally finding bugs, and to
> support that we should try to rearrange false positives so that smatch
> doesn't report on those.
>
> Perhaps there's some way we can do that here, without uglifying things?

Yes, we can make "unsigned int nr;" signed: "int nr;", should be OK.
Would you like me to send a fix patch with this change?

Thank you,
Pasha


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

* Re: [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
  2025-11-29 20:20     ` Pasha Tatashin
@ 2025-11-29 23:52       ` Andrew Morton
  2025-11-30  0:55         ` Pasha Tatashin
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2025-11-29 23:52 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

On Sat, 29 Nov 2025 15:20:51 -0500 Pasha Tatashin <pasha.tatashin@soleen.com> wrote:

> > > >    393  static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> > > >    394  {
> > > >    395          const struct luo_ioctl_op *op;
> > > >    396          struct luo_ucmd ucmd = {};
> > > >    397          union ucmd_buffer buf;
> > > >    398          unsigned int nr;
> > > >    399          int err;
> > > >    400
> > > >    401          nr = _IOC_NR(cmd);
> > > >  > 402          if (nr < LIVEUPDATE_CMD_BASE ||
> > > >    403              (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {
> > >
> > > This is a false positive. The logic is designed to work generically
> > > for any base, similar to how it is handled in session ioctl and
> > > iommufd ictl. The warning only triggers because LIVEUPDATE_CMD_BASE
> > > happens to be 0 in this specific case.
> >
> > This is a difficult situation.
> >
> > In some cases smatch has found a bug, in other cases it's just being
> > irritating.
> >
> > I think there is value in smatch occasionally finding bugs, and to
> > support that we should try to rearrange false positives so that smatch
> > doesn't report on those.
> >
> > Perhaps there's some way we can do that here, without uglifying things?
> 
> Yes, we can make "unsigned int nr;" signed: "int nr;", should be OK.
> Would you like me to send a fix patch with this change?

Sure, thanks.  It's a bit lame using an inappropriate type but the
world won't end if we do.


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

* Re: [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero.
  2025-11-29 23:52       ` Andrew Morton
@ 2025-11-30  0:55         ` Pasha Tatashin
  0 siblings, 0 replies; 6+ messages in thread
From: Pasha Tatashin @ 2025-11-30  0:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
	Mike Rapoport (Microsoft),
	Pratyush Yadav

On Sat, Nov 29, 2025 at 6:52 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sat, 29 Nov 2025 15:20:51 -0500 Pasha Tatashin <pasha.tatashin@soleen.com> wrote:
>
> > > > >    393  static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> > > > >    394  {
> > > > >    395          const struct luo_ioctl_op *op;
> > > > >    396          struct luo_ucmd ucmd = {};
> > > > >    397          union ucmd_buffer buf;
> > > > >    398          unsigned int nr;
> > > > >    399          int err;
> > > > >    400
> > > > >    401          nr = _IOC_NR(cmd);
> > > > >  > 402          if (nr < LIVEUPDATE_CMD_BASE ||
> > > > >    403              (nr - LIVEUPDATE_CMD_BASE) >= ARRAY_SIZE(luo_ioctl_ops)) {
> > > >
> > > > This is a false positive. The logic is designed to work generically
> > > > for any base, similar to how it is handled in session ioctl and
> > > > iommufd ictl. The warning only triggers because LIVEUPDATE_CMD_BASE
> > > > happens to be 0 in this specific case.
> > >
> > > This is a difficult situation.
> > >
> > > In some cases smatch has found a bug, in other cases it's just being
> > > irritating.
> > >
> > > I think there is value in smatch occasionally finding bugs, and to
> > > support that we should try to rearrange false positives so that smatch
> > > doesn't report on those.
> > >
> > > Perhaps there's some way we can do that here, without uglifying things?
> >
> > Yes, we can make "unsigned int nr;" signed: "int nr;", should be OK.
> > Would you like me to send a fix patch with this change?
>
> Sure, thanks.  It's a bit lame using an inappropriate type but the
> world won't end if we do.

I will send a patch.

Actually, I realized we can simply drop the nr < LIVEUPDATE_CMD_BASE
check entirely.

Since nr is unsigned, if it is less than LIVEUPDATE_CMD_BASE, the
subtraction (nr - LIVEUPDATE_CMD_BASE) will wrap around to a large
positive value. This will trigger the >= ARRAY_SIZE check
automatically. This solves the warning without needing to change nr to
a signed type.

Pasha


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

end of thread, other threads:[~2025-11-30  0:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-27 19:29 [linux-next:master 12118/12398] kernel/liveupdate/luo_core.c:402 luo_ioctl() warn: unsigned 'nr' is never less than zero kernel test robot
2025-11-29 19:51 ` Pasha Tatashin
2025-11-29 20:00   ` Andrew Morton
2025-11-29 20:20     ` Pasha Tatashin
2025-11-29 23:52       ` Andrew Morton
2025-11-30  0:55         ` Pasha Tatashin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox