hi,
> -----Original Message-----
> From: Parav Pandit <parav@xxxxxxxxxx>
> Sent: Thursday, October 31, 2024 12:15 PM
> To: Lege Wang <lege.wang@xxxxxxxxxxxxxxx>; virtualization@xxxxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx; mst@xxxxxxxxxx; jasowang@xxxxxxxxxx
> Subject: RE: [PATCH] vp_vdpa: fix id_table array not null terminated error
>
> External Mail: This email originated from OUTSIDE of the organization!
> Do not click links, open attachments or provide ANY information unless you
> recognize the sender and know the content is safe.
>
>
> > From: Xiaoguang Wang <lege.wang@xxxxxxxxxxxxxxx>
> > Sent: Thursday, October 31, 2024 9:35 AM
> >
> > Allocate one extra virtio_device_id as null terminator, otherwise
> > vdpa_mgmtdev_get_classes() may iterate multiple times and visit undefined
> > memory.
> >
> > Fixes: ffbda8e9df10 ("vdpa/vp_vdpa : add vdpa tool support in vp_vdpa")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Xiaoguang Wang <lege.wang@xxxxxxxxxxxxxxx>
> > ---
> > drivers/vdpa/virtio_pci/vp_vdpa.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > index ac4ab22f7d8b..74cc4ed77cc4 100644
> > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > @@ -612,7 +612,11 @@ static int vp_vdpa_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > goto mdev_err;
> > }
> >
> > - mdev_id = kzalloc(sizeof(struct virtio_device_id), GFP_KERNEL);
> > + /*
> > + * id_table should be a null terminated array.
> > + * See vdpa_mgmtdev_get_classes().
> > + */
> > + mdev_id = kzalloc(sizeof(struct virtio_device_id) * 2, GFP_KERNEL);
> Only one additional entry is needed for null termination. No need to allocate
> 2x memory.
> Even though you have only two entries. Reading code as +1 is better to
> understand null termination.
Sorry, I don't get your point here, vp_vdpa_probe only needs one struct virtio_device_id, plus
one null termination, "sizeof(struct virtio_device_id) * 2 " should be enough here?
>
> And for array, you should use,
> array = kcalloc(2, sizeof(mdev_id), GFP_KERNEL);
OK, thanks for your suggestion.
Regards,
Xiaoguang Wang
>
> > if (!mdev_id) {
> > err = -ENOMEM;
> > goto mdev_id_err;
> > --
> > 2.40.1
> >
