> > +static ssize_t vfio_pci_tph_uinfo_dup(struct vfio_pci_tph *tph,
> > + void __user *arg, size_t
> > argsz,
> > + struct vfio_pci_tph_info
> > **info)
> > +{
> > + size_t minsz;
> > +
> > + if (tph->count > VFIO_TPH_INFO_MAX)
> > + return -EINVAL;
> > + if (!tph->count)
> > + return 0;
> > +
> > + minsz = tph->count * sizeof(struct vfio_pci_tph_info);
> > + if (minsz < argsz)
> > + return -EINVAL;
> > +
> > + *info = memdup_user(arg, minsz);
>
> You can use memdup_array_user() instead of the lines above. It does the
> multiplication plus overflow check for you and will make your code more
> compact.
>
Thank you, wasn't aware of it.
> > + if (IS_ERR(info))
> > + return PTR_ERR(info);
> > +
> > + return minsz;
>
> see below…
>
> > +}
> > +
> > +static int vfio_pci_feature_tph_st_op(struct vfio_pci_core_device
> > *vdev,
> > + struct vfio_pci_tph *tph,
> > + void __user *arg, size_t
> > argsz)
> > +{
> > + int i, mtype, err = 0;
> > + u32 cpu_uid;
> > + struct vfio_pci_tph_info *info = NULL;
> > + ssize_t data_size = vfio_pci_tph_uinfo_dup(tph, arg, argsz,
> > &info);
> > +
> > + if (data_size <= 0)
> > + return data_size;
>
> So it seems you return here in case of an error. However, that would result in a
> length of 0 being an error?
>
> I would try to avoid to return 0 for an error whenever possible. That breaks
> convention.
>
> How about you return the result value of memdup_array_user() in …uinfo_dup()?
>
> The only thing I can't tell is whether tph->count == 0 should be treated as an
> error. Maybe map it to -EINVAL?
>
>
I wasn't sure before, but I lean towards -EINVAL now. I will amend this to -EINVAL.
I saw this like reading 0 bytes from a file descriptor, which returns 0.
But there are also differences, read return the number of bytes read, whereas this
doesn't return the number of records.
