linux-kernel - Re: [PATCH 1/2] vduse: avoid adding implicit padding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJaqyWd5b4vrRnrDS+fvQd+=vzmV=WhOL3ZahU_eh3_mwfNTtg@mail.gmail.com>
Date: Tue, 3 Feb 2026 08:00:23 +0100
From: Eugenio Perez Martin <eperezma@...hat.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Arnd Bergmann <arnd@...nel.org>, "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Xie Yongji <xieyongji@...edance.com>, 
	Anders Roxell <anders.roxell@...aro.org>, Marco Crivellari <marco.crivellari@...e.com>, 
	Ashwini Sahu <ashwini@...ig.com>, virtualization@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] vduse: avoid adding implicit padding

On Mon, Feb 2, 2026 at 1:07 PM Arnd Bergmann <arnd@...db.de> 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 <eperezma@...hat.com> 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?