On Mon, Feb 2, 2026 at 1:07 PM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Feb 2, 2026, at 12:50, Eugenio Perez Martin wrote:
> > On Mon, Feb 2, 2026 at 12:28 PM Eugenio Perez Martin <[email protected]>
> > wrote:
> >> > ret = -EFAULT;
> >> > - if (cmd == VDUSE_IOTLB_GET_FD2) {
> >> > - if (copy_from_user(&entry, argp, sizeof(entry)))
> >> > - break;
> >> > - } else {
> >> > - if (copy_from_user(&entry.v1, argp,
> >> > - sizeof(entry.v1)))
> >> > - break;
> >> > - }
> >> > + if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> >>
> >> I did not know about _IOC_SIZE and I like how it reduces the complexity,
> >> thanks!
> >>
> >> As a proposal, maybe we can add MIN(_IOC_SIZE, sizeof(entry)) ? Not
> >> sure if it is too much boilerplate for nothing as the compiler should
> >> make the code identical and the uapi ioctl part should never change.
> >> But it seems to me future changes to the code are better tied with the
> >> MIN.
> >> I'm ok with not including MIN() either way.
>
> I think it's more readable without the MIN(), but I don't mind
> adding it either.
>
Yep, I see how MIN() adds a little bit of noise. I'm happy either way :).
> >> > */
> >> > struct vduse_iotlb_entry_v2 {
> >> > - struct vduse_iotlb_entry v1;
> >> > + __u64 offset;
> >> > + __u64 start;
> >> > + __u64 last;
> >> > + __u8 perm;
> >> > + __u8 padding[7];
> >> > __u32 asid;
> >> > - __u32 reserved[12];
> >> > + __u32 reserved[11];
> >
> > (I hit "Send" too early).
> >
> > We could make this padding[3] so reserved keeps being [12]. This way
> > the struct members keep the same alignment between the commits. Not
> > super important as there should not be a lot of users of this right
> > now, we're just introducing it.
>
> I think that is too risky, as it would overlay 'asid' on top of
> previously uninitialized padding fields coming from userspace
> on most architectures. Since there was previously no is_mem_zero()
> check for the padding, I don't think it should be reused at all.
>
Ok fair point. Yes, this would need something in the code like:
if (cmd == VDUSE_IOTLB_GET_FD)
/* Ignoring whatever came in padding as it could be uninitialized due to
* not having this member in the struct definition
*/
memset(entry.padding, 0, sizeof(entry.padding);
else if (!mem_is_zero(entry.padding, sizeof(entry.padding,
sizeof(entry.padding))
/* Return error following the style of the rest of the code */
return -EINVAL
---
So we make sure we can use the padding in the future. Would that work?
