On Mon, Jun 15, 2026 at 04:45:25PM +0000, David Matlack wrote:
> 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 <[email protected]>
> > Reviewed-by: Vipin Sharma <[email protected]>
> > Signed-off-by: Alex Mastro <[email protected]>
> > ---
> > Changes in v2:
> > - Reverse xmas tree variable ordering
> > - Link to v1: 
> > https://patch.msgid.link/20260612-scratch-amastro-vfio-selftests-avoid-vlas-v1-1-ba3acb635...@fb.com
> > 
> > To: David Matlack <[email protected]>
> > To: Alex Williamson <[email protected]>
> > To: Shuah Khan <[email protected]>
> > To: Raghavendra Rao Ananta <[email protected]>
> > To: Vipin Sharma <[email protected]>
> > Cc: Shuah Khan <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> >  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())

SGTM

> 
> > +   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.

Good call, yes will do. Thanks for reviewing!

> 
> > +   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 <[email protected]>
> > 

Reply via email to