public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7
@ 2023-12-04  8:56 kernel test robot
  2023-12-04  9:02 ` Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2023-12-04  8:56 UTC (permalink / raw)
  To: Suwan Kim
  Cc: oe-kbuild-all, linux-kernel, Michael S. Tsirkin, Stefan Hajnoczi,
	Christoph Hellwig, Max Gurtovoy, Chaitanya Kulkarni

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   33cc938e65a98f1d29d0a18403dbbee050dcad9a
commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
date:   1 year, 6 months ago
config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/reproduce)

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/202312041509.DIyvEt9h-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/block/virtio_blk.c: In function 'init_vq':
>> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
     570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
         |                                                                    ^~
   drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
     570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
         |                                                          ^~~~~~~~~~~~~
   drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
     570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +570 drivers/block/virtio_blk.c

   511	
   512	static int init_vq(struct virtio_blk *vblk)
   513	{
   514		int err;
   515		int i;
   516		vq_callback_t **callbacks;
   517		const char **names;
   518		struct virtqueue **vqs;
   519		unsigned short num_vqs;
   520		unsigned int num_poll_vqs;
   521		struct virtio_device *vdev = vblk->vdev;
   522		struct irq_affinity desc = { 0, };
   523	
   524		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
   525					   struct virtio_blk_config, num_queues,
   526					   &num_vqs);
   527		if (err)
   528			num_vqs = 1;
   529	
   530		if (!err && !num_vqs) {
   531			dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
   532			return -EINVAL;
   533		}
   534	
   535		num_vqs = min_t(unsigned int,
   536				min_not_zero(num_request_queues, nr_cpu_ids),
   537				num_vqs);
   538	
   539		num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
   540	
   541		vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
   542		vblk->io_queues[HCTX_TYPE_READ] = 0;
   543		vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
   544	
   545		dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
   546					vblk->io_queues[HCTX_TYPE_DEFAULT],
   547					vblk->io_queues[HCTX_TYPE_READ],
   548					vblk->io_queues[HCTX_TYPE_POLL]);
   549	
   550		vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
   551		if (!vblk->vqs)
   552			return -ENOMEM;
   553	
   554		names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
   555		callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
   556		vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
   557		if (!names || !callbacks || !vqs) {
   558			err = -ENOMEM;
   559			goto out;
   560		}
   561	
   562		for (i = 0; i < num_vqs - num_poll_vqs; i++) {
   563			callbacks[i] = virtblk_done;
   564			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
   565			names[i] = vblk->vqs[i].name;
   566		}
   567	
   568		for (; i < num_vqs; i++) {
   569			callbacks[i] = NULL;
 > 570			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
   571			names[i] = vblk->vqs[i].name;
   572		}
   573	
   574		/* Discover virtqueues and write information to configuration.  */
   575		err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
   576		if (err)
   577			goto out;
   578	
   579		for (i = 0; i < num_vqs; i++) {
   580			spin_lock_init(&vblk->vqs[i].lock);
   581			vblk->vqs[i].vq = vqs[i];
   582		}
   583		vblk->num_vqs = num_vqs;
   584	
   585	out:
   586		kfree(vqs);
   587		kfree(callbacks);
   588		kfree(names);
   589		if (err)
   590			kfree(vblk->vqs);
   591		return err;
   592	}
   593	

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

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

* Re: drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7
  2023-12-04  8:56 drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 kernel test robot
@ 2023-12-04  9:02 ` Michael S. Tsirkin
  2023-12-04 13:30   ` Stefan Hajnoczi
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2023-12-04  9:02 UTC (permalink / raw)
  To: kernel test robot
  Cc: Suwan Kim, oe-kbuild-all, linux-kernel, Stefan Hajnoczi,
	Christoph Hellwig, Max Gurtovoy, Chaitanya Kulkarni, pbonzini

On Mon, Dec 04, 2023 at 04:56:35PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   33cc938e65a98f1d29d0a18403dbbee050dcad9a
> commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
> date:   1 year, 6 months ago
> config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/reproduce)
> 
> 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/202312041509.DIyvEt9h-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/block/virtio_blk.c: In function 'init_vq':
> >> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
>      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
>          |                                                                    ^~
>    drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
>      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
>          |                                                          ^~~~~~~~~~~~~
>    drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
>      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 
> vim +570 drivers/block/virtio_blk.c
> 
>    511	
>    512	static int init_vq(struct virtio_blk *vblk)
>    513	{
>    514		int err;
>    515		int i;
>    516		vq_callback_t **callbacks;
>    517		const char **names;
>    518		struct virtqueue **vqs;
>    519		unsigned short num_vqs;
>    520		unsigned int num_poll_vqs;
>    521		struct virtio_device *vdev = vblk->vdev;
>    522		struct irq_affinity desc = { 0, };
>    523	
>    524		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
>    525					   struct virtio_blk_config, num_queues,
>    526					   &num_vqs);
>    527		if (err)
>    528			num_vqs = 1;
>    529	
>    530		if (!err && !num_vqs) {
>    531			dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
>    532			return -EINVAL;
>    533		}
>    534	
>    535		num_vqs = min_t(unsigned int,
>    536				min_not_zero(num_request_queues, nr_cpu_ids),
>    537				num_vqs);
>    538	
>    539		num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
>    540	
>    541		vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
>    542		vblk->io_queues[HCTX_TYPE_READ] = 0;
>    543		vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
>    544	
>    545		dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
>    546					vblk->io_queues[HCTX_TYPE_DEFAULT],
>    547					vblk->io_queues[HCTX_TYPE_READ],
>    548					vblk->io_queues[HCTX_TYPE_POLL]);
>    549	
>    550		vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
>    551		if (!vblk->vqs)
>    552			return -ENOMEM;
>    553	
>    554		names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
>    555		callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
>    556		vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
>    557		if (!names || !callbacks || !vqs) {
>    558			err = -ENOMEM;
>    559			goto out;
>    560		}
>    561	
>    562		for (i = 0; i < num_vqs - num_poll_vqs; i++) {
>    563			callbacks[i] = virtblk_done;
>    564			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
>    565			names[i] = vblk->vqs[i].name;
>    566		}
>    567	
>    568		for (; i < num_vqs; i++) {
>    569			callbacks[i] = NULL;
>  > 570			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
>    571			names[i] = vblk->vqs[i].name;
>    572		}
>    573	
>    574		/* Discover virtqueues and write information to configuration.  */
>    575		err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
>    576		if (err)
>    577			goto out;
>    578	
>    579		for (i = 0; i < num_vqs; i++) {
>    580			spin_lock_init(&vblk->vqs[i].lock);
>    581			vblk->vqs[i].vq = vqs[i];
>    582		}
>    583		vblk->num_vqs = num_vqs;
>    584	
>    585	out:
>    586		kfree(vqs);
>    587		kfree(callbacks);
>    588		kfree(names);
>    589		if (err)
>    590			kfree(vblk->vqs);
>    591		return err;
>    592	}
>    593	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Stefan, Paolo,
It's a false positive but do we want to fix it? Make i unsigned?

-- 
MST


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

* Re: drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7
  2023-12-04  9:02 ` Michael S. Tsirkin
@ 2023-12-04 13:30   ` Stefan Hajnoczi
  0 siblings, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2023-12-04 13:30 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kernel test robot, Suwan Kim, oe-kbuild-all, linux-kernel,
	Christoph Hellwig, Max Gurtovoy, Chaitanya Kulkarni, pbonzini

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

On Mon, Dec 04, 2023 at 04:02:07AM -0500, Michael S. Tsirkin wrote:
> On Mon, Dec 04, 2023 at 04:56:35PM +0800, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   33cc938e65a98f1d29d0a18403dbbee050dcad9a
> > commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
> > date:   1 year, 6 months ago
> > config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/config)
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/202312041509.DIyvEt9h-lkp@intel.com/reproduce)
> > 
> > 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/202312041509.DIyvEt9h-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    drivers/block/virtio_blk.c: In function 'init_vq':
> > >> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
> >      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> >          |                                                                    ^~
> >    drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
> >      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> >          |                                                          ^~~~~~~~~~~~~
> >    drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
> >      570 |                 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> >          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > 
> > vim +570 drivers/block/virtio_blk.c
> > 
> >    511	
> >    512	static int init_vq(struct virtio_blk *vblk)
> >    513	{
> >    514		int err;
> >    515		int i;
> >    516		vq_callback_t **callbacks;
> >    517		const char **names;
> >    518		struct virtqueue **vqs;
> >    519		unsigned short num_vqs;
> >    520		unsigned int num_poll_vqs;
> >    521		struct virtio_device *vdev = vblk->vdev;
> >    522		struct irq_affinity desc = { 0, };
> >    523	
> >    524		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> >    525					   struct virtio_blk_config, num_queues,
> >    526					   &num_vqs);
> >    527		if (err)
> >    528			num_vqs = 1;
> >    529	
> >    530		if (!err && !num_vqs) {
> >    531			dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
> >    532			return -EINVAL;
> >    533		}
> >    534	
> >    535		num_vqs = min_t(unsigned int,
> >    536				min_not_zero(num_request_queues, nr_cpu_ids),
> >    537				num_vqs);
> >    538	
> >    539		num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
> >    540	
> >    541		vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
> >    542		vblk->io_queues[HCTX_TYPE_READ] = 0;
> >    543		vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
> >    544	
> >    545		dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
> >    546					vblk->io_queues[HCTX_TYPE_DEFAULT],
> >    547					vblk->io_queues[HCTX_TYPE_READ],
> >    548					vblk->io_queues[HCTX_TYPE_POLL]);
> >    549	
> >    550		vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
> >    551		if (!vblk->vqs)
> >    552			return -ENOMEM;
> >    553	
> >    554		names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
> >    555		callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
> >    556		vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
> >    557		if (!names || !callbacks || !vqs) {
> >    558			err = -ENOMEM;
> >    559			goto out;
> >    560		}
> >    561	
> >    562		for (i = 0; i < num_vqs - num_poll_vqs; i++) {
> >    563			callbacks[i] = virtblk_done;
> >    564			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
> >    565			names[i] = vblk->vqs[i].name;
> >    566		}
> >    567	
> >    568		for (; i < num_vqs; i++) {
> >    569			callbacks[i] = NULL;
> >  > 570			snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> >    571			names[i] = vblk->vqs[i].name;
> >    572		}
> >    573	
> >    574		/* Discover virtqueues and write information to configuration.  */
> >    575		err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
> >    576		if (err)
> >    577			goto out;
> >    578	
> >    579		for (i = 0; i < num_vqs; i++) {
> >    580			spin_lock_init(&vblk->vqs[i].lock);
> >    581			vblk->vqs[i].vq = vqs[i];
> >    582		}
> >    583		vblk->num_vqs = num_vqs;
> >    584	
> >    585	out:
> >    586		kfree(vqs);
> >    587		kfree(callbacks);
> >    588		kfree(names);
> >    589		if (err)
> >    590			kfree(vblk->vqs);
> >    591		return err;
> >    592	}
> >    593	
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
> 
> Stefan, Paolo,
> It's a false positive but do we want to fix it? Make i unsigned?

It's a false positive:

That maximum number of virtqueues is 65535. That's 5 characters. The
maximum value of num_poll_vqs is actually 65534 because it's capped at
num_vqs - 1.

The warning reveals that the analysis thinks i can be negative:

  drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]

It can't because of num_poll_vqs = min_t(unsigned int, poll_queues,
num_vqs - 1).

VQ_NAME_LEN is 16, so 9 bytes are used for "req_poll." and the NUL
terminator:

  snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);

9 + 5 = 14 is always less than VQ_NAME_LEN (16), so it looks like a
false positive.

That said, it's still worth cleaning up the types because they are
inconsistent:

  unsigned short num_vqs;
  unsigned int num_poll_vqs;

and using "%d" for an unsigned value also contributes to the problem.

I'll send a patch.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2023-12-04 13:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04  8:56 drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 kernel test robot
2023-12-04  9:02 ` Michael S. Tsirkin
2023-12-04 13:30   ` Stefan Hajnoczi

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