Re: [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32

[Brian Gerst]

On Tue, Dec 1, 2020 at 12:34 PM Andy Lutomirski <luto@xxxxxxxxxx> wrote:
>
> On Tue, Dec 1, 2020 at 9:23 AM Andy Lutomirski <luto@xxxxxxxxxx> wrote:
> >
> > On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@xxxxxxxxx> wrote:
> > >
> > > Commit 121b32a58a3a converted native x86-32 which take 64-bit arguments to
> > > use the compat handlers to allow conversion to passing args via pt_regs.
> > > sys_fanotify_mark() was however missed, as it has a general compat handler.
> > > Add a config option that will use the syscall wrapper that takes the split
> > > args for native 32-bit.
> > >
> > > Reported-by: Paweł Jasiak <pawel@xxxxxxxxxx>
> > > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > > Signed-off-by: Brian Gerst <brgerst@xxxxxxxxx>
> > > ---
> > >  arch/Kconfig                       |  6 ++++++
> > >  arch/x86/Kconfig                   |  1 +
> > >  fs/notify/fanotify/fanotify_user.c | 17 +++++++----------
> > >  include/linux/syscalls.h           | 24 ++++++++++++++++++++++++
> > >  4 files changed, 38 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 090ef3566c56..452cc127c285 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -1045,6 +1045,12 @@ config HAVE_STATIC_CALL_INLINE
> > >         bool
> > >         depends on HAVE_STATIC_CALL
> > >
> > > +config ARCH_SPLIT_ARG64
> > > +       bool
> > > +       help
> > > +          If a 32-bit architecture requires 64-bit arguments to be split into
> > > +          pairs of 32-bit arguemtns, select this option.
> >
> > You misspelled arguments.  You might also want to clarify that, for
> > 64-bit arches, this means that compat syscalls split their arguments.
>
> No, that's backwards.  Maybe it should be depends !64BIT instead.
>
> But I'm really quite confused about something: what's special about
> x86 here?

x86 is special because of the pt_regs-based syscall interface.  It
would be nice to get all arches to that point eventually.

> Are there really Linux arches (compat or 32-bit native)
> that *don't* split arguments like this?  Sure, some arches probably
> work the same way that x86 used to in which the compiler did the
> splitting by magic for us, but that was always a bit of a kludge.
> Could this change maybe be made unconditional?

It probably can be made unconditional.  That will take some research
on which arches have the implicit alignment requirement.  From looking
at the existing compat handlers, ARM, MIPS, and PowerPC 32-bit ABIs
need alignment.

--
Brian Gerst