The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: David Matlack <dmatlack@google.com>
To: Alex Mastro <amastro@fb.com>
Cc: Alex Williamson <alex@shazbot.org>, Shuah Khan <shuah@kernel.org>,
	Raghavendra Rao Ananta <rananta@google.com>,
	Vipin Sharma <vipinsh@google.com>,
	kvm@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v2] selftests/vfio: avoid VLAs
Date: Mon, 15 Jun 2026 16:45:25 +0000	[thread overview]
Message-ID: <ajAsJXWxRQtkIaCf@google.com> (raw)
In-Reply-To: <20260615-scratch-amastro-vfio-selftests-avoid-vlas-v2-1-4e4df462908a@fb.com>

On 2026-06-15 08:47 AM, Alex Mastro wrote:

Please follow the local convention for shortlogs:

  vfio: selftests: Description of the commit

> Allocate VFIO ioctl requests dynamically instead of using VLAs. GCC 11.5.0
> rejects initialized VLAs with:
> 
>   error: variable-sized object may not be initialized
> 
> The replaced stack u8 arrays also do not guarantee native struct alignment
> for the aliased pointers.
> 
> Fixes: 19faf6fd969c ("vfio: selftests: Add a helper library for VFIO selftests")
> Fixes: 20face8c75ff ("vfio: selftests: Add helper to set/override a vf_token")
> Assisted-by: Codex:gpt-5.5-high
> Tested-by: Vipin Sharma <vipinsh@google.com>
> Reviewed-by: Vipin Sharma <vipinsh@google.com>
> Signed-off-by: Alex Mastro <amastro@fb.com>
> ---
> Changes in v2:
> - Reverse xmas tree variable ordering
> - Link to v1: https://patch.msgid.link/20260612-scratch-amastro-vfio-selftests-avoid-vlas-v1-1-ba3acb635f0a@fb.com
> 
> To: David Matlack <dmatlack@google.com>
> To: Alex Williamson <alex@shazbot.org>
> To: Shuah Khan <shuah@kernel.org>
> To: Raghavendra Rao Ananta <rananta@google.com>
> To: Vipin Sharma <vipinsh@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: kvm@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  tools/testing/selftests/vfio/lib/vfio_pci_device.c | 28 +++++++++++++---------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 94dc5fcecbeb..c65d5ed33e51 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -30,13 +30,12 @@
>  static void vfio_pci_irq_set(struct vfio_pci_device *device,
>  			     u32 index, u32 vector, u32 count, int *fds)
>  {
> -	u8 buf[sizeof(struct vfio_irq_set) + sizeof(int) * count];
> -	struct vfio_irq_set *irq = (void *)&buf;
> -	int *irq_fds = (void *)&irq->data;
> +	size_t irq_size = sizeof(struct vfio_irq_set) + sizeof(int) * count;

optional nit: s/irq_size/argsz/ (same for vfio_device_feature_ioctl())

> +	struct vfio_irq_set *irq;
>  
> -	memset(buf, 0, sizeof(buf));
> -
> -	irq->argsz = sizeof(buf);
> +	irq = calloc(1, irq_size);
> +	VFIO_ASSERT_NOT_NULL(irq);

Could you add a precursor patch to add a calloc_assert() helper to
assert.h next to ioctl_assert() and snprintf_assert()? It looks like
there's quite a bit of code that repeats the
calloc()/VFIO_ASSERT_NOT_NULL() pattern.

> +	irq->argsz = irq_size;
>  	irq->flags = VFIO_IRQ_SET_ACTION_TRIGGER;
>  	irq->index = index;
>  	irq->start = vector;
> @@ -44,12 +43,13 @@ static void vfio_pci_irq_set(struct vfio_pci_device *device,
>  
>  	if (count) {
>  		irq->flags |= VFIO_IRQ_SET_DATA_EVENTFD;
> -		memcpy(irq_fds, fds, sizeof(int) * count);
> +		memcpy(irq->data, fds, sizeof(int) * count);
>  	} else {
>  		irq->flags |= VFIO_IRQ_SET_DATA_NONE;
>  	}
>  
>  	ioctl_assert(device->fd, VFIO_DEVICE_SET_IRQS, irq);
> +	free(irq);
>  }
>  
>  void vfio_pci_irq_trigger(struct vfio_pci_device *device, u32 index, u32 vector)
> @@ -118,15 +118,21 @@ static void vfio_pci_irq_get(struct vfio_pci_device *device, u32 index,
>  static int vfio_device_feature_ioctl(int fd, u32 flags, void *data,
>  				     size_t data_size)
>  {
> -	u8 buffer[sizeof(struct vfio_device_feature) + data_size] = {};
> -	struct vfio_device_feature *feature = (void *)buffer;
> +	size_t feature_size = sizeof(struct vfio_device_feature) + data_size;
> +	struct vfio_device_feature *feature;
> +	int ret;
>  
> +	feature = calloc(1, feature_size);
> +	VFIO_ASSERT_NOT_NULL(feature);
>  	memcpy(feature->data, data, data_size);
>  
> -	feature->argsz = sizeof(buffer);
> +	feature->argsz = feature_size;
>  	feature->flags = flags;
>  
> -	return ioctl(fd, VFIO_DEVICE_FEATURE, feature);
> +	ret = ioctl(fd, VFIO_DEVICE_FEATURE, feature);
> +	free(feature);
> +
> +	return ret;
>  }
>  
>  static void vfio_device_feature_set(int fd, u16 feature, void *data, size_t data_size)
> 
> ---
> base-commit: a26b499b757cfc8bbff1088bb1b844639e250893
> change-id: 20260612-scratch-amastro-vfio-selftests-avoid-vlas-395eb3dcb3ab
> 
> Best regards,
> --  
> Alex Mastro <amastro@fb.com>
> 

  reply	other threads:[~2026-06-15 16:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-15 15:47 [PATCH v2] selftests/vfio: avoid VLAs Alex Mastro
2026-06-15 16:45 ` David Matlack [this message]
2026-06-15 17:13   ` Alex Mastro

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=ajAsJXWxRQtkIaCf@google.com \
    --to=dmatlack@google.com \
    --cc=alex@shazbot.org \
    --cc=amastro@fb.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=rananta@google.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=vipinsh@google.com \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox