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

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

[Brian Gerst]

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@jasiak.xyz>
Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 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.
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e4499b01ae9a..41189d3de9fb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -19,6 +19,7 @@ config X86_32
 	select KMAP_LOCAL
 	select MODULES_USE_ELF_REL
 	select OLD_SIGACTION
+	select ARCH_SPLIT_ARG64
 
 config X86_64
 	def_bool y
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 3e01d8f2ab90..dcab112e1f00 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1285,26 +1285,23 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	return ret;
 }
 
+#ifndef CONFIG_ARCH_SPLIT_ARG64
 SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 			      __u64, mask, int, dfd,
 			      const char  __user *, pathname)
 {
 	return do_fanotify_mark(fanotify_fd, flags, mask, dfd, pathname);
 }
+#endif
 
-#ifdef CONFIG_COMPAT
-COMPAT_SYSCALL_DEFINE6(fanotify_mark,
+#if defined(CONFIG_ARCH_SPLIT_ARG64) || defined(CONFIG_COMPAT)
+SYSCALL32_DEFINE6(fanotify_mark,
 				int, fanotify_fd, unsigned int, flags,
-				__u32, mask0, __u32, mask1, int, dfd,
+				SC_ARG64(mask), int, dfd,
 				const char  __user *, pathname)
 {
-	return do_fanotify_mark(fanotify_fd, flags,
-#ifdef __BIG_ENDIAN
-				((__u64)mask0 << 32) | mask1,
-#else
-				((__u64)mask1 << 32) | mask0,
-#endif
-				 dfd, pathname);
+	return do_fanotify_mark(fanotify_fd, flags, SC_VAL64(__u64, mask),
+				dfd, pathname);
 }
 #endif
 
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 37bea07c12f2..aea0ce9f3b74 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -251,6 +251,30 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
 	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
 #endif /* __SYSCALL_DEFINEx */
 
+/* For split 64-bit arguments on 32-bit architectures */
+#ifdef __LITTLE_ENDIAN
+#define SC_ARG64(name) u32, name##_lo, u32, name##_hi
+#else
+#define SC_ARG64(name) u32, name##_hi, u32, name##_lo
+#endif
+#define SC_VAL64(type, name) ((type) name##_hi << 32 | name##_lo)
+
+#ifdef CONFIG_COMPAT
+#define SYSCALL32_DEFINE1 COMPAT_SYSCALL_DEFINE1
+#define SYSCALL32_DEFINE2 COMPAT_SYSCALL_DEFINE2
+#define SYSCALL32_DEFINE3 COMPAT_SYSCALL_DEFINE3
+#define SYSCALL32_DEFINE4 COMPAT_SYSCALL_DEFINE4
+#define SYSCALL32_DEFINE5 COMPAT_SYSCALL_DEFINE5
+#define SYSCALL32_DEFINE6 COMPAT_SYSCALL_DEFINE6
+#else
+#define SYSCALL32_DEFINE1 SYSCALL_DEFINE1
+#define SYSCALL32_DEFINE2 SYSCALL_DEFINE2
+#define SYSCALL32_DEFINE3 SYSCALL_DEFINE3
+#define SYSCALL32_DEFINE4 SYSCALL_DEFINE4
+#define SYSCALL32_DEFINE5 SYSCALL_DEFINE5
+#define SYSCALL32_DEFINE6 SYSCALL_DEFINE6
+#endif
+
 /*
  * Called before coming back to user-mode. Returning to user-mode with an
  * address limit different than USER_DS can allow to overwrite kernel memory.
-- 
2.26.2

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

[Jan Kara]

On Mon 30-11-20 17:30:59, Brian Gerst 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@jasiak.xyz>
> Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> Signed-off-by: Brian Gerst <brgerst@gmail.com>

Thanks for the patch! It looks good to me. Feel free to add:

Acked-by: Jan Kara <jack@suse.cz>

I assume you plan to push this via x86 tree given the changes are mostly
there, don't you?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Borislav Petkov]

On Tue, Dec 01, 2020 at 10:48:10AM +0100, Jan Kara wrote:
> On Mon 30-11-20 17:30:59, Brian Gerst 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@jasiak.xyz>
> > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> 
> Thanks for the patch! It looks good to me. Feel free to add:
> 
> Acked-by: Jan Kara <jack@suse.cz>
> 
> I assume you plan to push this via x86 tree given the changes are mostly
> there, don't you?

Looks sane to me too, I guess I can send it to Linus even now so that it
lands in 5.10. Is that what you'd prefer Jan?

Also, Paweł, can you give that one a try to see if it fixes your case?
It should...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Andy Lutomirski]

On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@gmail.com> 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@jasiak.xyz>
> Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> ---
>  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.

Aside from that:

Acked-by: Andy Lutomirski <luto@kernel.org>

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

[Andy Lutomirski]

On Tue, Dec 1, 2020 at 9:23 AM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@gmail.com> 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@jasiak.xyz>
> > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > ---
> >  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?  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?

--Andy

>
> Aside from that:
>
> Acked-by: Andy Lutomirski <luto@kernel.org>

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

[Catalin Marinas]

On Tue, Dec 01, 2020 at 09:34:32AM -0800, Andy Lutomirski wrote:
> On Tue, Dec 1, 2020 at 9:23 AM Andy Lutomirski <luto@kernel.org> wrote:
> > On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@gmail.com> 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@jasiak.xyz>
> > > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > > ---
> > >  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?  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.

On arm32 we rely on the compiler splitting a 64-bit argument in two
consecutive registers. But I wouldn't say it's a kludge (well, mostly)
as that's part of the arm procedure calling standard. Currently arm32
doesn't pass the syscall arguments through a read from pt_regs, so all
is handled transparently.

On arm64 compat, we need to re-assemble the arguments with some
wrappers explicitly (arch/arm64/kernel/sys32.c) or call the generic
wrapper like in the compat_sys_fanotify_mark() case.

> Could this change maybe be made unconditional?

I think it's fine in this particular case.

I don't think it's valid in general because of the arm (and maybe
others) requirement that the first register of a 64-bit argument is an
even number (IIRC, Russell should know better). If the u64 mask was an
argument before or after the current position, the compiler would have
introduced a pad register but not if the arg is split in two u32.

-- 
Catalin

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

[Andy Lutomirski]

On Tue, Dec 1, 2020 at 11:00 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, Dec 01, 2020 at 09:34:32AM -0800, Andy Lutomirski wrote:
> > On Tue, Dec 1, 2020 at 9:23 AM Andy Lutomirski <luto@kernel.org> wrote:
> > > On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@gmail.com> 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@jasiak.xyz>
> > > > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > > > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > > > ---
> > > >  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?  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.
>
> On arm32 we rely on the compiler splitting a 64-bit argument in two
> consecutive registers. But I wouldn't say it's a kludge (well, mostly)
> as that's part of the arm procedure calling standard. Currently arm32
> doesn't pass the syscall arguments through a read from pt_regs, so all
> is handled transparently.
>
> On arm64 compat, we need to re-assemble the arguments with some
> wrappers explicitly (arch/arm64/kernel/sys32.c) or call the generic
> wrapper like in the compat_sys_fanotify_mark() case.
>
> > Could this change maybe be made unconditional?
>
> I think it's fine in this particular case.
>
> I don't think it's valid in general because of the arm (and maybe
> others) requirement that the first register of a 64-bit argument is an
> even number (IIRC, Russell should know better). If the u64 mask was an
> argument before or after the current position, the compiler would have
> introduced a pad register but not if the arg is split in two u32.
>

So I guess Brian's macro is more like "this is a 32-bit arch that
needs to split 64-bit syscall args but naively splitting them is
correct", which is true on x86_32 but not necessarily on arm.

Should we consider having a real program that runs as part of the
build generate the syscall wrappers?  The logic involved is pushing
the bounds of C macro magic and human comprehension.

--Andy

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@kernel.org> wrote:
>
> On Tue, Dec 1, 2020 at 9:23 AM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Mon, Nov 30, 2020 at 2:31 PM Brian Gerst <brgerst@gmail.com> 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@jasiak.xyz>
> > > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > > ---
> > >  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

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

[Jan Kara]

On Tue 01-12-20 16:51:26, Borislav Petkov wrote:
> On Tue, Dec 01, 2020 at 10:48:10AM +0100, Jan Kara wrote:
> > On Mon 30-11-20 17:30:59, Brian Gerst 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@jasiak.xyz>
> > > Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> > > Signed-off-by: Brian Gerst <brgerst@gmail.com>
> > 
> > Thanks for the patch! It looks good to me. Feel free to add:
> > 
> > Acked-by: Jan Kara <jack@suse.cz>
> > 
> > I assume you plan to push this via x86 tree given the changes are mostly
> > there, don't you?
> 
> Looks sane to me too, I guess I can send it to Linus even now so that it
> lands in 5.10. Is that what you'd prefer Jan?

Yes, that would be fine by me. Although I don't think there's a huge rush.
The thing is broken for some time already so if it goes in later with CC to
stable, that would also work OK.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[tip: x86/urgent] fanotify: Fix sys_fanotify_mark() on native x86-32

[tip-bot2 for Brian Gerst]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     2ca408d9c749c32288bc28725f9f12ba30299e8f
Gitweb:        https://git.kernel.org/tip/2ca408d9c749c32288bc28725f9f12ba30299e8f
Author:        Brian Gerst <brgerst@gmail.com>
AuthorDate:    Mon, 30 Nov 2020 17:30:59 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 28 Dec 2020 11:58:59 +01:00

fanotify: Fix sys_fanotify_mark() on native x86-32

Commit

  121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")

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.

 [ bp: Fix typo in Kconfig help text. ]

Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
Reported-by: Paweł Jasiak <pawel@jasiak.xyz>
Signed-off-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Jan Kara <jack@suse.cz>
Acked-by: Andy Lutomirski <luto@kernel.org>
Link: https://lkml.kernel.org/r/20201130223059.101286-1-brgerst@gmail.com
---
 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 78c6f05..24862d1 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1105,6 +1105,12 @@ config HAVE_ARCH_PFN_VALID
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	bool
 
+config ARCH_SPLIT_ARG64
+	bool
+	help
+	   If a 32-bit architecture requires 64-bit arguments to be split into
+	   pairs of 32-bit arguments, select this option.
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7b6dd10..21f8511 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -19,6 +19,7 @@ config X86_32
 	select KMAP_LOCAL
 	select MODULES_USE_ELF_REL
 	select OLD_SIGACTION
+	select ARCH_SPLIT_ARG64
 
 config X86_64
 	def_bool y
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 3e01d8f..dcab112 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1285,26 +1285,23 @@ fput_and_out:
 	return ret;
 }
 
+#ifndef CONFIG_ARCH_SPLIT_ARG64
 SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 			      __u64, mask, int, dfd,
 			      const char  __user *, pathname)
 {
 	return do_fanotify_mark(fanotify_fd, flags, mask, dfd, pathname);
 }
+#endif
 
-#ifdef CONFIG_COMPAT
-COMPAT_SYSCALL_DEFINE6(fanotify_mark,
+#if defined(CONFIG_ARCH_SPLIT_ARG64) || defined(CONFIG_COMPAT)
+SYSCALL32_DEFINE6(fanotify_mark,
 				int, fanotify_fd, unsigned int, flags,
-				__u32, mask0, __u32, mask1, int, dfd,
+				SC_ARG64(mask), int, dfd,
 				const char  __user *, pathname)
 {
-	return do_fanotify_mark(fanotify_fd, flags,
-#ifdef __BIG_ENDIAN
-				((__u64)mask0 << 32) | mask1,
-#else
-				((__u64)mask1 << 32) | mask0,
-#endif
-				 dfd, pathname);
+	return do_fanotify_mark(fanotify_fd, flags, SC_VAL64(__u64, mask),
+				dfd, pathname);
 }
 #endif
 
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f3929af..7688bc9 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -251,6 +251,30 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
 	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
 #endif /* __SYSCALL_DEFINEx */
 
+/* For split 64-bit arguments on 32-bit architectures */
+#ifdef __LITTLE_ENDIAN
+#define SC_ARG64(name) u32, name##_lo, u32, name##_hi
+#else
+#define SC_ARG64(name) u32, name##_hi, u32, name##_lo
+#endif
+#define SC_VAL64(type, name) ((type) name##_hi << 32 | name##_lo)
+
+#ifdef CONFIG_COMPAT
+#define SYSCALL32_DEFINE1 COMPAT_SYSCALL_DEFINE1
+#define SYSCALL32_DEFINE2 COMPAT_SYSCALL_DEFINE2
+#define SYSCALL32_DEFINE3 COMPAT_SYSCALL_DEFINE3
+#define SYSCALL32_DEFINE4 COMPAT_SYSCALL_DEFINE4
+#define SYSCALL32_DEFINE5 COMPAT_SYSCALL_DEFINE5
+#define SYSCALL32_DEFINE6 COMPAT_SYSCALL_DEFINE6
+#else
+#define SYSCALL32_DEFINE1 SYSCALL_DEFINE1
+#define SYSCALL32_DEFINE2 SYSCALL_DEFINE2
+#define SYSCALL32_DEFINE3 SYSCALL_DEFINE3
+#define SYSCALL32_DEFINE4 SYSCALL_DEFINE4
+#define SYSCALL32_DEFINE5 SYSCALL_DEFINE5
+#define SYSCALL32_DEFINE6 SYSCALL_DEFINE6
+#endif
+
 /*
  * Called before coming back to user-mode. Returning to user-mode with an
  * address limit different than USER_DS can allow to overwrite kernel memory.