* Re: [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 [not found] ` <CALCETrWZ5eH=0Rjd-vBFRtk-tFQ3tN8_rReaKdVbSm78PFQ7_g@mail.gmail.com> @ 2020-12-01 17:34 ` Andy Lutomirski 2020-12-01 19:00 ` Catalin Marinas 2020-12-01 19:19 ` Brian Gerst 0 siblings, 2 replies; 4+ messages in thread From: Andy Lutomirski @ 2020-12-01 17:34 UTC (permalink / raw) To: Andy Lutomirski, linux-arch Cc: Brian Gerst, LKML, X86 ML, Borislav Petkov, Thomas Gleixner, Jan Kara, Paweł Jasiak 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 2020-12-01 17:34 ` [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 Andy Lutomirski @ 2020-12-01 19:00 ` Catalin Marinas 2020-12-01 19:10 ` Andy Lutomirski 2020-12-01 19:19 ` Brian Gerst 1 sibling, 1 reply; 4+ messages in thread From: Catalin Marinas @ 2020-12-01 19:00 UTC (permalink / raw) To: Andy Lutomirski Cc: linux-arch, Brian Gerst, LKML, X86 ML, Borislav Petkov, Thomas Gleixner, Jan Kara, Paweł Jasiak, Russell King 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 2020-12-01 19:00 ` Catalin Marinas @ 2020-12-01 19:10 ` Andy Lutomirski 0 siblings, 0 replies; 4+ messages in thread From: Andy Lutomirski @ 2020-12-01 19:10 UTC (permalink / raw) To: Catalin Marinas Cc: Andy Lutomirski, linux-arch, Brian Gerst, LKML, X86 ML, Borislav Petkov, Thomas Gleixner, Jan Kara, Paweł Jasiak, Russell King 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 2020-12-01 17:34 ` [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 Andy Lutomirski 2020-12-01 19:00 ` Catalin Marinas @ 2020-12-01 19:19 ` Brian Gerst 1 sibling, 0 replies; 4+ messages in thread From: Brian Gerst @ 2020-12-01 19:19 UTC (permalink / raw) To: Andy Lutomirski Cc: linux-arch, LKML, X86 ML, Borislav Petkov, Thomas Gleixner, Jan Kara, Paweł Jasiak 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-01 19:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20201130223059.101286-1-brgerst@gmail.com>
[not found] ` <CALCETrWZ5eH=0Rjd-vBFRtk-tFQ3tN8_rReaKdVbSm78PFQ7_g@mail.gmail.com>
2020-12-01 17:34 ` [PATCH] fanotify: Fix sys_fanotify_mark() on native x86-32 Andy Lutomirski
2020-12-01 19:00 ` Catalin Marinas
2020-12-01 19:10 ` Andy Lutomirski
2020-12-01 19:19 ` Brian Gerst
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).