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.
>> > */
>> > 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.
Arnd
