All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Eugenio Pérez" <eperezma@redhat.com>,
	"Arnd Bergmann" <arnd@kernel.org>,
	"Jason Wang" <jasowang@redhat.com>,
	"Xie Yongji" <xieyongji@bytedance.com>,
	"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
	"Anders Roxell" <anders.roxell@linaro.org>,
	"Marco Crivellari" <marco.crivellari@suse.com>,
	virtualization@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
Date: Mon, 2 Feb 2026 11:45:13 -0500	[thread overview]
Message-ID: <20260202114412-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <30db98bd-8361-46f5-aec1-ffbc4e8574dd@app.fastmail.com>

On Mon, Feb 02, 2026 at 12:59:03PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 2, 2026, at 12:34, Eugenio Perez Martin wrote:
> > On Mon, Feb 2, 2026 at 11:07 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> These two ioctls are incompatible on 32-bit x86 userspace, because
> >> the data structures are shorter than they are on 64-bit.
> >>
> >> Add compad handling to the regular ioctl handler to just handle
> >> them the same way and ignore the extra padding. This could be
> >> done in a separate .compat_ioctl handler, but the main one already
> >> handles two versions of VDUSE_IOTLB_GET_FD, so adding a third one
> >> fits in rather well.
> >>
> >
> > I'm just learning about the COMPAT_ stuff but does this mean the
> > userland app need to call a different ioctl depending if it is
> > compiled for 32 bits or 64 bits? I guess it is not the case, but how
> > is that handled?
> 
> In a definition like
> 
> #define VDUSE_IOTLB_GET_FD      _IOWR(VDUSE_BASE, 0x10, struct vduse_iotlb_entry)
> 
> The resulting integer value encodes sizeof(struct vduse_iotlb_entry)
> in some of the bits. Since x86-32 and x86-64 have different
> sizes for this particular structure, the command codes are
> different for the same macro. The recommendation from 
> Documentation/driver-api/ioctl.rst is to use structures with
> a consistent layout across all architectures to avoid that.
> 
> The normal way to handle this once it has gone wrong is to split
> out the actual handler into a function that takes the kernel
> structure, and a .compat_ioctl() handler that copies the
> 32-bit structure to the stack in the correct format.
> 
> Since the v1 structures here are /almost/ compatible aside from
> the padding at the end, my patch here takes a shortcut and does
> not add a custom .compat_ioctl handler but instead changes
> the native version on x86-64 to deal with both layouts.
> This does mean that the kernel driver now also accepts the
> 64-bit layout coming from compat tasks, and the compat layout
> coming from 64-bit tasks.
> 
> Nothing in userspace changes, as it still just uses the existing
> VDUSE_IOTLB_GET_FD macro, and the kernel continues to handle
> the native layout as before.
> 
>     Arnd

I think .compat_ioctl would be cleaner frankly. Just look at
all the ifdefery. And who knows what broken-ness userspace
comes up with with this approach. Better use the standard approach.


  reply	other threads:[~2026-02-02 16:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-02  9:59 [PATCH 1/2] vduse: avoid adding implicit padding Arnd Bergmann
2026-02-02  9:59 ` [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
2026-02-02 11:34   ` Eugenio Perez Martin
2026-02-02 11:59     ` Arnd Bergmann
2026-02-02 16:45       ` Michael S. Tsirkin [this message]
2026-02-02 22:54         ` Arnd Bergmann
2026-02-03 10:35           ` Michael S. Tsirkin
2026-02-03 14:13             ` Eugenio Perez Martin
2026-02-03 14:41               ` Arnd Bergmann
2026-02-03 15:03                 ` Eugenio Perez Martin
2026-02-02 11:28 ` [PATCH 1/2] vduse: avoid adding implicit padding Eugenio Perez Martin
2026-02-02 11:50   ` Eugenio Perez Martin
2026-02-02 12:06     ` Arnd Bergmann
2026-02-03  7:00       ` Eugenio Perez Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260202114412-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=anders.roxell@linaro.org \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marco.crivellari@suse.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xieyongji@bytedance.com \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.