I am trying to run examples from man fanotify.7 but fanotify_mark always
fail with errno = EFAULT.
fanotify_mark declaration is
SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
__u64, mask, int, dfd,
const char __user *, pathname)
When
fanotify_mark(4, FAN_MARK_ADD | FAN_MARK_ONLYDIR,
FAN_CREATE | FAN_ONDIR, AT_FDCWD, 0xdeadc0de)
is called on kernel side I can see in do_syscall_32_irqs_on that CPU
context is
bx = 0x4 = 4
cx = 0x9 = FAN_MARK_ADD | FAN_MARK_ONLYDIR,
dx = 0x40000100 = FAN_CREATE | FAN_ONDIR
si = 0x0
di = 0xffffff9c = AT_FDCWD
bp = 0xdeadc0de
ax = 0xffffffda
orix_ax = 0x153
I am not sure if it is ok because third argument is uint64_t so if I
understand correctly mask should be divided into two registers (dx and
si).
But in fanotify_mark we get
fanotify_fd = 4 = bx
flags = 0x9 = cx
mask = 0x40000100 = dx
dfd = 0 = si
pathname = 0xffffff9c = di
I believe that correct order is
fanotify_fd = 4 = bx
flags = 0x9 = cx
mask = 0x40000100 = (si << 32) | dx
dfd = 0xffffff9c = di
pathname = 0xdeadc0de = bp
I think that we should call COMPAT version of fanotify_mark here
COMPAT_SYSCALL_DEFINE6(fanotify_mark,
int, fanotify_fd, unsigned int, flags,
__u32, mask0, __u32, mask1, int, dfd,
const char __user *, pathname)
or something wrong is with 64-bits arguments.
I am running Linux 5.9.2 i686 on Pentium III (Coppermine).
For tests I am using Debian sid on qemu with 5.9.2 and default kernel
from repositories.
Everything works fine on 5.5 and 5.4.
--
Paweł Jasiak
On Sun, Nov 01, 2020 at 10:27:38PM +0100, Paweł Jasiak wrote:
> I am trying to run examples from man fanotify.7 but fanotify_mark always
> fail with errno = EFAULT.
>
> fanotify_mark declaration is
>
> SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> __u64, mask, int, dfd,
> const char __user *, pathname)
Don't worry about that. You aren't calling the SYSCALL, you're calling
glibc and glibc is turning it into a syscall.
extern int fanotify_mark (int __fanotify_fd, unsigned int __flags,
uint64_t __mask, int __dfd, const char *__pathname)
> When
>
> fanotify_mark(4, FAN_MARK_ADD | FAN_MARK_ONLYDIR,
> FAN_CREATE | FAN_ONDIR, AT_FDCWD, 0xdeadc0de)
The last argument is supposed to be a pointer to a string. I'm guessing
there's no string at 0xdeadc0de.
On 01/11/20, Matthew Wilcox wrote:
> On Sun, Nov 01, 2020 at 10:27:38PM +0100, Paweł Jasiak wrote:
> > I am trying to run examples from man fanotify.7 but fanotify_mark always
> > fail with errno = EFAULT.
> >
> > fanotify_mark declaration is
> >
> > SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> > __u64, mask, int, dfd,
> > const char __user *, pathname)
>
> Don't worry about that. You aren't calling the SYSCALL, you're calling
> glibc and glibc is turning it into a syscall.
>
> extern int fanotify_mark (int __fanotify_fd, unsigned int __flags,
> uint64_t __mask, int __dfd, const char *__pathname)
>
> > When
> >
> > fanotify_mark(4, FAN_MARK_ADD | FAN_MARK_ONLYDIR,
> > FAN_CREATE | FAN_ONDIR, AT_FDCWD, 0xdeadc0de)
>
> The last argument is supposed to be a pointer to a string. I'm guessing
> there's no string at 0xdeadc0de.
You are right but it's not a problem. 0xdeadc0de is just a _well
known_ address here only for debug purpose.
pathname inside kernel should be a pointer to string located in
user space at 0xdeadc0de but it is equal to 0xffffff9c which is
AT_FDCWD.
If you call
fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_ONLYDIR, FAN_CREATE |
FAN_ONDIR, AT_FDCWD, argv[1]);
from example with *valid* pointer at argv[1] you still get EFAULT
because pathname is equal to AT_FDCWD in kernel space -- last argument
is not used.
In my example in user space we have
fanotify_fd = 4
flags = 0x9
mask = 0x40000100
dfd = 0xffffff9c
pathname = 0xdeadc0de
and in kernel space we have
fanotify_fd = 4
flags = 0x9
mask = 0x40000100
dfd = 0
pathname = 0xffffff9c
So all arguments after __u64 mask are shifted by one.
It looks similar to https://lists.linux.it/pipermail/ltp/2020-June/017436.html
--
Paweł Jasiak
On Sun 01-11-20 22:27:38, Paweł Jasiak wrote:
> I am trying to run examples from man fanotify.7 but fanotify_mark always
> fail with errno = EFAULT.
>
> fanotify_mark declaration is
>
> SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> __u64, mask, int, dfd,
> const char __user *, pathname)
>
> When
>
> fanotify_mark(4, FAN_MARK_ADD | FAN_MARK_ONLYDIR,
> FAN_CREATE | FAN_ONDIR, AT_FDCWD, 0xdeadc0de)
>
> is called on kernel side I can see in do_syscall_32_irqs_on that CPU
> context is
> bx = 0x4 = 4
> cx = 0x9 = FAN_MARK_ADD | FAN_MARK_ONLYDIR,
> dx = 0x40000100 = FAN_CREATE | FAN_ONDIR
> si = 0x0
> di = 0xffffff9c = AT_FDCWD
> bp = 0xdeadc0de
> ax = 0xffffffda
> orix_ax = 0x153
>
> I am not sure if it is ok because third argument is uint64_t so if I
> understand correctly mask should be divided into two registers (dx and
> si).
>
> But in fanotify_mark we get
> fanotify_fd = 4 = bx
> flags = 0x9 = cx
> mask = 0x40000100 = dx
> dfd = 0 = si
> pathname = 0xffffff9c = di
>
> I believe that correct order is
> fanotify_fd = 4 = bx
> flags = 0x9 = cx
> mask = 0x40000100 = (si << 32) | dx
> dfd = 0xffffff9c = di
> pathname = 0xdeadc0de = bp
>
> I think that we should call COMPAT version of fanotify_mark here
>
> COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> int, fanotify_fd, unsigned int, flags,
> __u32, mask0, __u32, mask1, int, dfd,
> const char __user *, pathname)
>
> or something wrong is with 64-bits arguments.
>
> I am running Linux 5.9.2 i686 on Pentium III (Coppermine).
> For tests I am using Debian sid on qemu with 5.9.2 and default kernel
> from repositories.
>
> Everything works fine on 5.5 and 5.4.
Strange. Thanks for report. Looks like some issue got created / exposed
somewhere between 5.5 and 5.9 (actually probably between 5.5 and 5.7
because the Linaro report you mentioned [1] is from 5.7-rc6). There were
no changes in this area in fanotify, I think it must have been some x86
change that triggered this. Hum, looking into x86 changelog in that time
range there was a series rewriting 32-bit ABI [2] that got merged into
5.7-rc1. Can you perhaps check whether 5.6 is good and 5.7-rc1 is bad?
Brian, any idea whether your series could regress fanotify_mark(2) syscall?
Do we have somewhere documented which syscalls need compat wrappers and how
they should look like?
Honza
[1] https://lists.linux.it/pipermail/ltp/2020-June/017436.html
[2] https://lore.kernel.org/lkml/[email protected]/
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 02/11/20, Jan Kara wrote:
> Strange. Thanks for report. Looks like some issue got created / exposed
> somewhere between 5.5 and 5.9 (actually probably between 5.5 and 5.7
> because the Linaro report you mentioned [1] is from 5.7-rc6). There were
> no changes in this area in fanotify, I think it must have been some x86
> change that triggered this. Hum, looking into x86 changelog in that time
> range there was a series rewriting 32-bit ABI [2] that got merged into
> 5.7-rc1. Can you perhaps check whether 5.6 is good and 5.7-rc1 is bad?
5.6 works.
5.7-rc1 doesn't work.
--
Paweł Jasiak
I have written small patch that fixes problem for me and doesn't break
x86_64.
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 3e01d8f2ab90..cf0b97309975 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1285,12 +1285,27 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
return ret;
}
+#if defined(CONFIG_X86) && !defined(CONFIG_64BIT)
+SYSCALL_DEFINE6(fanotify_mark,
+ int, fanotify_fd, unsigned int, flags, __u32, mask0,
+ __u32, mask1, 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);
+}
+#else
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,
--
Paweł Jasiak
On Tue 03-11-20 22:17:47, Paweł Jasiak wrote:
> I have written small patch that fixes problem for me and doesn't break
> x86_64.
Yeah, that looks sensible, thanks for the patch. But I'm waiting for some
explanation from x86 folks when compat handlers are really needed and why
it wasn't needed before syscall wrapper rewrite in 5.7-rc1 and is needed
now. Brian, Andy, Thomas?
Honza
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 3e01d8f2ab90..cf0b97309975 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1285,12 +1285,27 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> return ret;
> }
>
> +#if defined(CONFIG_X86) && !defined(CONFIG_64BIT)
> +SYSCALL_DEFINE6(fanotify_mark,
> + int, fanotify_fd, unsigned int, flags, __u32, mask0,
> + __u32, mask1, 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);
> +}
> +#else
> 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,
>
>
> --
>
> Paweł Jasiak
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Tue 03-11-20 22:17:47, Paweł Jasiak wrote:
> I have written small patch that fixes problem for me and doesn't break
> x86_64.
OK, with a help of Boris Petkov I think I have a fix that looks correct
(attach). Can you please try whether it works for you? Thanks!
Honza
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 3e01d8f2ab90..cf0b97309975 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1285,12 +1285,27 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> return ret;
> }
>
> +#if defined(CONFIG_X86) && !defined(CONFIG_64BIT)
> +SYSCALL_DEFINE6(fanotify_mark,
> + int, fanotify_fd, unsigned int, flags, __u32, mask0,
> + __u32, mask1, 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);
> +}
> +#else
> 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,
>
>
> --
>
> Paweł Jasiak
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 23/11/20, Jan Kara wrote:
> OK, with a help of Boris Petkov I think I have a fix that looks correct
> (attach). Can you please try whether it works for you? Thanks!
Unfortunately I am getting a linker error.
ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
> From fc9104a50a774ec198c1e3a145372cde77df7967 Mon Sep 17 00:00:00 2001
> From: Jan Kara <[email protected]>
> Date: Mon, 23 Nov 2020 17:37:00 +0100
> Subject: [PATCH] fanotify: Fix fanotify_mark() on 32-bit archs
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Commit converting syscalls taking 64-bit arguments to new scheme of compat
> handlers omitted converting fanotify_mark(2) which then broke the
> syscall for 32-bit ABI. Add missed conversion.
>
> CC: Brian Gerst <[email protected]>
> Suggested-by: Borislav Petkov <[email protected]>
> Reported-by: Paweł Jasiak <[email protected]>
> Reported-by: Naresh Kamboju <[email protected]>
> Fixes: 121b32a58a3a ("x86/entry/32: Use IA32-specific wrappers for syscalls taking 64-bit arguments")
> CC: [email protected]
> Signed-off-by: Jan Kara <[email protected]>
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 2 +-
> fs/notify/fanotify/fanotify_user.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 0d0667a9fbd7..b2ec6ff88307 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -350,7 +350,7 @@
> 336 i386 perf_event_open sys_perf_event_open
> 337 i386 recvmmsg sys_recvmmsg_time32 compat_sys_recvmmsg_time32
> 338 i386 fanotify_init sys_fanotify_init
> -339 i386 fanotify_mark sys_fanotify_mark compat_sys_fanotify_mark
> +339 i386 fanotify_mark sys_ia32_fanotify_mark
> 340 i386 prlimit64 sys_prlimit64
> 341 i386 name_to_handle_at sys_name_to_handle_at
> 342 i386 open_by_handle_at sys_open_by_handle_at compat_sys_open_by_handle_at
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 3e01d8f2ab90..e20e7b53a87f 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1293,7 +1293,7 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
> }
>
> #ifdef CONFIG_COMPAT
> -COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> +SYSCALL_DEFINE6(ia32_fanotify_mark,
> int, fanotify_fd, unsigned int, flags,
> __u32, mask0, __u32, mask1, int, dfd,
> const char __user *, pathname)
> --
> 2.16.4
>
--
Paweł Jasiak
Hi Jan,
I love your patch! Yet something to improve:
[auto build test ERROR on ext3/fsnotify]
[also build test ERROR on linux/master linus/master tip/x86/asm v5.10-rc5 next-20201123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jan-Kara/fanotify-Fix-fanotify_mark-on-32-bit-archs/20201124-004903
base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
config: i386-randconfig-s001-20201123 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-134-gb59dbdaf-dirty
# https://github.com/0day-ci/linux/commit/db2e8841f458509774f8dcbc5df73be0a91b80d0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jan-Kara/fanotify-Fix-fanotify_mark-on-32-bit-archs/20201124-004903
git checkout db2e8841f458509774f8dcbc5df73be0a91b80d0
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> On 23/11/20, Jan Kara wrote:
> > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > (attach). Can you please try whether it works for you? Thanks!
>
> Unfortunately I am getting a linker error.
>
> ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
Because CONFIG_COMPAT is not set in your .config.
Jan, look at 121b32a58a3a and especially this hunk
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 9b294c13809a..b8f89f78b8cd 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -53,6 +53,8 @@ obj-y += setup.o x86_init.o i8259.o irqinit.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o
obj-$(CONFIG_IRQ_WORK) += irq_work.o
obj-y += probe_roms.o
+obj-$(CONFIG_X86_32) += sys_ia32.o
+obj-$(CONFIG_IA32_EMULATION) += sys_ia32.o
how it enables the ia32 syscalls depending on those config items. Now,
you have
#ifdef CONFIG_COMPAT
-COMPAT_SYSCALL_DEFINE6(fanotify_mark,
+SYSCALL_DEFINE6(ia32_fanotify_mark,
which is under CONFIG_COMPAT which is not set in Paweł's config.
config COMPAT
def_bool y
depends on IA32_EMULATION || X86_X32
but it depends on those two config items.
However, it looks to me like ia32_fanotify_mark's definition should be
simply under CONFIG_X86_32 because:
IA32_EMULATION is 32-bit emulation on 64-bit kernels
X86_X32 is for the x32 ABI
But I could be missing an angle...
HTH.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue 24-11-20 09:45:07, Borislav Petkov wrote:
> On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> > On 23/11/20, Jan Kara wrote:
> > > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > > (attach). Can you please try whether it works for you? Thanks!
> >
> > Unfortunately I am getting a linker error.
> >
> > ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
>
> Because CONFIG_COMPAT is not set in your .config.
>
> Jan, look at 121b32a58a3a and especially this hunk
>
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 9b294c13809a..b8f89f78b8cd 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -53,6 +53,8 @@ obj-y += setup.o x86_init.o i8259.o irqinit.o
> obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> obj-$(CONFIG_IRQ_WORK) += irq_work.o
> obj-y += probe_roms.o
> +obj-$(CONFIG_X86_32) += sys_ia32.o
> +obj-$(CONFIG_IA32_EMULATION) += sys_ia32.o
>
> how it enables the ia32 syscalls depending on those config items. Now,
> you have
>
> #ifdef CONFIG_COMPAT
> -COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> +SYSCALL_DEFINE6(ia32_fanotify_mark,
>
> which is under CONFIG_COMPAT which is not set in Paweł's config.
>
> config COMPAT
> def_bool y
> depends on IA32_EMULATION || X86_X32
>
> but it depends on those two config items.
>
> However, it looks to me like ia32_fanotify_mark's definition should be
> simply under CONFIG_X86_32 because:
>
> IA32_EMULATION is 32-bit emulation on 64-bit kernels
> X86_X32 is for the x32 ABI
Thanks for checking! I didn't realize I needed to change the ifdefs as well
(I missed that bit in 121b32a58a3a). So do I understand correctly that
whenever the kernel is 64-bit, 64-bit syscall args (e.g. defined as u64) are
passed just fine regardless of whether the userspace is 32-bit or not?
Also how about other 32-bit archs? Because I now realized that
CONFIG_COMPAT as well as the COMPAT_SYSCALL_DEFINE6() is also utilized by
other 32-bit archs (I can see a reference to compat_sys_fanotify_mark e.g.
in sparc, powerpc, and other args). So I probably need to actually keep
that for other archs but do the modification only for x86, don't I?
So something like attached patch?
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Tue, Nov 24, 2020 at 11:20:33AM +0100, Jan Kara wrote:
> On Tue 24-11-20 09:45:07, Borislav Petkov wrote:
> > On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> > > On 23/11/20, Jan Kara wrote:
> > > > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > > > (attach). Can you please try whether it works for you? Thanks!
> > >
> > > Unfortunately I am getting a linker error.
> > >
> > > ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
> >
> > Because CONFIG_COMPAT is not set in your .config.
> >
> > Jan, look at 121b32a58a3a and especially this hunk
> >
> > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> > index 9b294c13809a..b8f89f78b8cd 100644
> > --- a/arch/x86/kernel/Makefile
> > +++ b/arch/x86/kernel/Makefile
> > @@ -53,6 +53,8 @@ obj-y += setup.o x86_init.o i8259.o irqinit.o
> > obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> > obj-$(CONFIG_IRQ_WORK) += irq_work.o
> > obj-y += probe_roms.o
> > +obj-$(CONFIG_X86_32) += sys_ia32.o
> > +obj-$(CONFIG_IA32_EMULATION) += sys_ia32.o
> >
> > how it enables the ia32 syscalls depending on those config items. Now,
> > you have
> >
> > #ifdef CONFIG_COMPAT
> > -COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> > +SYSCALL_DEFINE6(ia32_fanotify_mark,
> >
> > which is under CONFIG_COMPAT which is not set in Paweł's config.
> >
> > config COMPAT
> > def_bool y
> > depends on IA32_EMULATION || X86_X32
> >
> > but it depends on those two config items.
> >
> > However, it looks to me like ia32_fanotify_mark's definition should be
> > simply under CONFIG_X86_32 because:
> >
> > IA32_EMULATION is 32-bit emulation on 64-bit kernels
> > X86_X32 is for the x32 ABI
>
> Thanks for checking! I didn't realize I needed to change the ifdefs as well
> (I missed that bit in 121b32a58a3a). So do I understand correctly that
> whenever the kernel is 64-bit, 64-bit syscall args (e.g. defined as u64) are
> passed just fine regardless of whether the userspace is 32-bit or not?
>
> Also how about other 32-bit archs? Because I now realized that
> CONFIG_COMPAT as well as the COMPAT_SYSCALL_DEFINE6() is also utilized by
> other 32-bit archs (I can see a reference to compat_sys_fanotify_mark e.g.
> in sparc, powerpc, and other args). So I probably need to actually keep
> that for other archs but do the modification only for x86, don't I?
Hmm, you raise a good point and looking at that commit again, the
intention is to supply those ia32 wrappers as both 32-bit native *and*
32-bit emulation ones.
So I think this
> -#ifdef CONFIG_COMPAT
> +#if defined(CONFIG_COMPAT) || defined(CONFIG_X86_32)
> +#ifdef CONFIG_X86_32
> +SYSCALL_DEFINE6(ia32_fanotify_mark,
> +#elif CONFIG_COMPAT
> COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> +#endif
should be
if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
SYSCALL_DEFINE6(ia32_fanotify_mark,
#elif CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE6(fanotify_mark,
#endif
or so.
Meaning that 32-bit native or 32-bit emulation supplies
ia32_fanotify_mark() as a syscall wrapper and other arches doing
CONFIG_COMPAT, still do the COMPAT_SYSCALL_DEFINE6() thing.
But I'd prefer if someone more knowledgeable than me in that whole
syscall macros muck to have a look...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, 24 Nov 2020 at 15:50, Jan Kara <[email protected]> wrote:
>
> On Tue 24-11-20 09:45:07, Borislav Petkov wrote:
> > On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> > > On 23/11/20, Jan Kara wrote:
> > > > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > > > (attach). Can you please try whether it works for you? Thanks!
> > >
<trim>
>
> Thanks for checking! I didn't realize I needed to change the ifdefs as well
> (I missed that bit in 121b32a58a3a). So do I understand correctly that
> whenever the kernel is 64-bit, 64-bit syscall args (e.g. defined as u64) are
> passed just fine regardless of whether the userspace is 32-bit or not?
>
> Also how about other 32-bit archs? Because I now realized that
> CONFIG_COMPAT as well as the COMPAT_SYSCALL_DEFINE6() is also utilized by
> other 32-bit archs (I can see a reference to compat_sys_fanotify_mark e.g.
> in sparc, powerpc, and other args). So I probably need to actually keep
> that for other archs but do the modification only for x86, don't I?
>
> So something like attached patch?
I have tested the attached patch on i386 and qemu_i386 and the reported problem
got fixed.
Test links,
https://lkft.validation.linaro.org/scheduler/job/1985236#L1176
https://lkft.validation.linaro.org/scheduler/job/1985238#L801
--
Linaro LKFT
https://lkft.linaro.org
On Thu, Nov 26, 2020 at 11:48:27AM +0100, Jan Kara wrote:
> I'd prefer that as well but if nobody pops up, I'll just push this to my
> tree next week and will see what breaks :)
Right. You could send a proper patch and Cc the usual suspects as now it
is buried in some thread which people might not read.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue 24-11-20 11:28:14, Borislav Petkov wrote:
> On Tue, Nov 24, 2020 at 11:20:33AM +0100, Jan Kara wrote:
> > On Tue 24-11-20 09:45:07, Borislav Petkov wrote:
> > > On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> > > > On 23/11/20, Jan Kara wrote:
> > > > > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > > > > (attach). Can you please try whether it works for you? Thanks!
> > > >
> > > > Unfortunately I am getting a linker error.
> > > >
> > > > ld: arch/x86/entry/syscall_32.o:(.rodata+0x54c): undefined reference to `__ia32_sys_ia32_fanotify_mark'
> > >
> > > Because CONFIG_COMPAT is not set in your .config.
> > >
> > > Jan, look at 121b32a58a3a and especially this hunk
> > >
> > > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> > > index 9b294c13809a..b8f89f78b8cd 100644
> > > --- a/arch/x86/kernel/Makefile
> > > +++ b/arch/x86/kernel/Makefile
> > > @@ -53,6 +53,8 @@ obj-y += setup.o x86_init.o i8259.o irqinit.o
> > > obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> > > obj-$(CONFIG_IRQ_WORK) += irq_work.o
> > > obj-y += probe_roms.o
> > > +obj-$(CONFIG_X86_32) += sys_ia32.o
> > > +obj-$(CONFIG_IA32_EMULATION) += sys_ia32.o
> > >
> > > how it enables the ia32 syscalls depending on those config items. Now,
> > > you have
> > >
> > > #ifdef CONFIG_COMPAT
> > > -COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> > > +SYSCALL_DEFINE6(ia32_fanotify_mark,
> > >
> > > which is under CONFIG_COMPAT which is not set in Paweł's config.
> > >
> > > config COMPAT
> > > def_bool y
> > > depends on IA32_EMULATION || X86_X32
> > >
> > > but it depends on those two config items.
> > >
> > > However, it looks to me like ia32_fanotify_mark's definition should be
> > > simply under CONFIG_X86_32 because:
> > >
> > > IA32_EMULATION is 32-bit emulation on 64-bit kernels
> > > X86_X32 is for the x32 ABI
> >
> > Thanks for checking! I didn't realize I needed to change the ifdefs as well
> > (I missed that bit in 121b32a58a3a). So do I understand correctly that
> > whenever the kernel is 64-bit, 64-bit syscall args (e.g. defined as u64) are
> > passed just fine regardless of whether the userspace is 32-bit or not?
> >
> > Also how about other 32-bit archs? Because I now realized that
> > CONFIG_COMPAT as well as the COMPAT_SYSCALL_DEFINE6() is also utilized by
> > other 32-bit archs (I can see a reference to compat_sys_fanotify_mark e.g.
> > in sparc, powerpc, and other args). So I probably need to actually keep
> > that for other archs but do the modification only for x86, don't I?
>
> Hmm, you raise a good point and looking at that commit again, the
> intention is to supply those ia32 wrappers as both 32-bit native *and*
> 32-bit emulation ones.
>
> So I think this
>
> > -#ifdef CONFIG_COMPAT
> > +#if defined(CONFIG_COMPAT) || defined(CONFIG_X86_32)
> > +#ifdef CONFIG_X86_32
> > +SYSCALL_DEFINE6(ia32_fanotify_mark,
> > +#elif CONFIG_COMPAT
> > COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> > +#endif
>
> should be
>
> if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> SYSCALL_DEFINE6(ia32_fanotify_mark,
> #elif CONFIG_COMPAT
> COMPAT_SYSCALL_DEFINE6(fanotify_mark,
> #endif
>
> or so.
>
> Meaning that 32-bit native or 32-bit emulation supplies
> ia32_fanotify_mark() as a syscall wrapper and other arches doing
> CONFIG_COMPAT, still do the COMPAT_SYSCALL_DEFINE6() thing.
Yeah, looking again at what those config options mean I agree. Patch
updated.
> But I'd prefer if someone more knowledgeable than me in that whole
> syscall macros muck to have a look...
I'd prefer that as well but if nobody pops up, I'll just push this to my
tree next week and will see what breaks :)
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu 26-11-20 01:01:30, Naresh Kamboju wrote:
> On Tue, 24 Nov 2020 at 15:50, Jan Kara <[email protected]> wrote:
> >
> > On Tue 24-11-20 09:45:07, Borislav Petkov wrote:
> > > On Mon, Nov 23, 2020 at 11:46:51PM +0100, Paweł Jasiak wrote:
> > > > On 23/11/20, Jan Kara wrote:
> > > > > OK, with a help of Boris Petkov I think I have a fix that looks correct
> > > > > (attach). Can you please try whether it works for you? Thanks!
> > > >
> <trim>
> >
> > Thanks for checking! I didn't realize I needed to change the ifdefs as well
> > (I missed that bit in 121b32a58a3a). So do I understand correctly that
> > whenever the kernel is 64-bit, 64-bit syscall args (e.g. defined as u64) are
> > passed just fine regardless of whether the userspace is 32-bit or not?
> >
> > Also how about other 32-bit archs? Because I now realized that
> > CONFIG_COMPAT as well as the COMPAT_SYSCALL_DEFINE6() is also utilized by
> > other 32-bit archs (I can see a reference to compat_sys_fanotify_mark e.g.
> > in sparc, powerpc, and other args). So I probably need to actually keep
> > that for other archs but do the modification only for x86, don't I?
> >
> > So something like attached patch?
>
> I have tested the attached patch on i386 and qemu_i386 and the reported problem
> got fixed.
>
> Test links,
> https://lkft.validation.linaro.org/scheduler/job/1985236#L1176
> https://lkft.validation.linaro.org/scheduler/job/1985238#L801
Thanks for testing! I've added your tested-by tag.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR