[LTP] [PATCH] fix fanotify syscall check on compat kernel

[LTP] [PATCH] fix fanotify syscall check on compat kernel

[Helge Deller]

The fanotify[0-5] testcases use the myfanotify_mark() wrapper for the
fanotify_mark() syscall. But the #define does not take into account, that
the mask field is actually of type _u64 where we need to split the hi
and low word in the syscall on a 32bit arch.

The attached patch converts the #define to a inline function where
the compiler will help to convert the given values to the correct types
(e.g. unsigned long).

Bug found and tested on the parisc/hppa (32bit userspace with 64bit
kernel) arch.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index a4c2a8f..b13cb7b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -28,12 +28,26 @@
 #ifndef	__FANOTIFY_H__
 #define	__FANOTIFY_H__
 
+#include <sys/types.h>
+#include <endian.h>
+#include "config.h"
+#include "linux_syscall_numbers.h"
+
 /* fanotify(7) wrappers */
 
 #define	myfanotify_init(flags, event_f_flags) \
 	syscall(__NR_fanotify_init, flags, event_f_flags)
 
-#define	myfanotify_mark(fd, flags, mask, dfd, pathname)	\
-	syscall(__NR_fanotify_mark, fd, flags, mask, dfd, pathname)
+long myfanotify_mark(int fd, unsigned int flags, unsigned long long mask, int dfd, const char *pathname)
+{
+#if __WORDSIZE == 64
+	return ltp_syscall(__NR_fanotify_mark, fd, flags, mask, dfd, pathname);
+#else
+	return ltp_syscall(__NR_fanotify_mark, fd, flags,
+			 __LONG_LONG_PAIR((unsigned long) (mask >> 32),
+					  (unsigned long) mask),
+			 dfd, (unsigned long) pathname);
+#endif
+}
 
 #endif /* __FANOTIFY_H__ */

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
>  #ifndef	__FANOTIFY_H__
>  #define	__FANOTIFY_H__
>  
> +#include <sys/types.h>
> +#include <endian.h>
> +#include "config.h"
> +#include "linux_syscall_numbers.h"
> +
>  /* fanotify(7) wrappers */
>  
>  #define	myfanotify_init(flags, event_f_flags) \
>  	syscall(__NR_fanotify_init, flags, event_f_flags)
>  
> -#define	myfanotify_mark(fd, flags, mask, dfd, pathname)	\
> -	syscall(__NR_fanotify_mark, fd, flags, mask, dfd, pathname)
> +long myfanotify_mark(int fd, unsigned int flags, unsigned long long mask, int dfd, const char *pathname)
> +{
> +#if __WORDSIZE == 64
> +	return ltp_syscall(__NR_fanotify_mark, fd, flags, mask, dfd, pathname);
> +#else
> +	return ltp_syscall(__NR_fanotify_mark, fd, flags,
> +			 __LONG_LONG_PAIR((unsigned long) (mask >> 32),
> +					  (unsigned long) mask),
> +			 dfd, (unsigned long) pathname);
> +#endif
> +}

I think that the x32 ABI should be handled here as well (as in the
fallocate.h). Mike can you confirm?

Also I've been unable to make the testcases work with -m32 on x86_64
both with and without the patch and even when using glibc wrappers from
sys/fallocate.h, in all cases the error is EINVAL which suggets that the
parameters are passed wrongly somewhere on the way.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
> > I think that the x32 ABI should be handled here as well (as in the
> > fallocate.h). Mike can you confirm?
> 
> Yes, I think so.
> I even thought of changing the #if to C-Coding like 
>   if ((sizeof(void*) == 8)
>     call 64bit_syscall
>    else
>     call 32bit_syscall_wrapper;

That would be still wrong on the x32 ABI where pointers are 32 bit but
syscalls get 64 bit parameters.

> I'm not sure if this can avoid the ifdefs though...

We should probably move the ifdef condition from the fallocate.h header
to some generic top-level header and use it in the syscall wrappers.

Something like:

#define LTP_USE_64_ABI __WORDSIZE == 64 || ....

Then we can use as:

#if LTP_USE_64_ABI
 ...
#else
 ...
#endif

> > Also I've been unable to make the testcases work with -m32 on x86_64
> > both with and without the patch and even when using glibc wrappers from
> > sys/fallocate.h, in all cases the error is EINVAL which suggets that the
> > parameters are passed wrongly somewhere on the way.
> 
> Does it work on the same kernel with -m64 ?
> In any case, for the tests to work the kernel needs to have been compiled with the
> kernel option CONFIG_FANOTIFY_ACCESS_PERMISSIONS set. In my initial tests this
> option wasn't set and I got EINVAL too.

It does work when compiled natively, i.e. without passing the -m32 to
the compiler commandline.

Something is wrong but I haven't yet found what.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
> > Also I've been unable to make the testcases work with -m32 on x86_64
> > both with and without the patch and even when using glibc wrappers from
> > sys/fallocate.h, in all cases the error is EINVAL which suggets that the
> > parameters are passed wrongly somewhere on the way.
> 
> Does it work on the same kernel with -m64 ?
> In any case, for the tests to work the kernel needs to have been compiled with the
> kernel option CONFIG_FANOTIFY_ACCESS_PERMISSIONS set. In my initial tests this
> option wasn't set and I got EINVAL too.

And it seems to work (-m32 with your patch) on two testing machines with
slightly older kernel. So I guess that the patch is fine and I had just
bad luck with one particular machine.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
> > > Also I've been unable to make the testcases work with -m32 on x86_64
> > > both with and without the patch and even when using glibc wrappers from
> > > sys/fallocate.h, in all cases the error is EINVAL which suggets that the
> > > parameters are passed wrongly somewhere on the way.
> > 
> > Does it work on the same kernel with -m64 ?
> > In any case, for the tests to work the kernel needs to have been compiled with the
> > kernel option CONFIG_FANOTIFY_ACCESS_PERMISSIONS set. In my initial tests this
> > option wasn't set and I got EINVAL too.
> 
> And it seems to work (-m32 with your patch) on two testing machines with
> slightly older kernel. So I guess that the patch is fine and I had just
> bad luck with one particular machine.

Found the cause:

commit 91c2e0bcae72a3086c698b5de2b950b885abb0e6
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Tue Mar 5 20:10:59 2013 -0500

    unify compat fanotify_mark(2), switch to COMPAT_SYSCALL_DEFINE
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>


Which was fixed in:

commit 592f6b842f64e416c7598a1b97c649b34241e22d
Author: Heiko Carstens <heiko.carstens@de.ibm.com>
Date:   Mon Jan 27 17:07:19 2014 -0800

    compat: fix sys_fanotify_mark
    
    Commit 91c2e0bcae72 ("unify compat fanotify_mark(2), switch to
    COMPAT_SYSCALL_DEFINE") added a new unified compat fanotify_mark syscall
    to be used by all architectures.
    
    Unfortunately the unified version merges the split mask parameter in a
    wrong way: the lower and higher word got swapped.
    
    This was discovered with glibc's tst-fanotify test case.


So kernels between 3.10 and 3.14 are broken.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
> The fanotify[0-5] testcases use the myfanotify_mark() wrapper for the
> fanotify_mark() syscall. But the #define does not take into account, that
> the mask field is actually of type _u64 where we need to split the hi
> and low word in the syscall on a 32bit arch.
> 
> The attached patch converts the #define to a inline function where
> the compiler will help to convert the given values to the correct types
> (e.g. unsigned long).
> 
> Bug found and tested on the parisc/hppa (32bit userspace with 64bit
> kernel) arch.

I've pushed slightly modified version of this patch. Please check that
it works fine for you.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fix fanotify syscall check on compat kernel

[chrubis]

Hi!
> Attached patch helps to find the reason when the fanotify03 testcase
> fails. For the test to work correctly, the kernel option
> CONFIG_FANOTIFY_ACCESS_PERMISSIONS needs to be enabled.

Pushed with minor change, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list