From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Subject: Re: [PATCH] fix fanotify_mark() breakage on big endian 32bit kernel Date: Fri, 04 Jul 2014 18:48:33 +0200 Message-ID: <53B6DAE1.1040709@gmx.de> References: <20140704151235.GA22454@ls3530.dhcp.wdf.sap.corp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Cc: Eric Paris , Linux Kernel , Heiko Carstens , linux-parisc@vger.kernel.org, James Bottomley To: Helge Deller Return-path: In-Reply-To: <20140704151235.GA22454@ls3530.dhcp.wdf.sap.corp> List-ID: List-Id: linux-parisc.vger.kernel.org On 04.07.2014 17:12, Helge Deller wrote: > This patch affects big endian architectures only. > > On those with 32bit userspace and 64bit kernel (CONFIG_COMPAT=y) the > 64bit mask parameter is correctly constructed out of two 32bit values in > the compat_fanotify_mark() function and then passed as 64bit parameter > to the fanotify_mark() syscall. > > But for the CONFIG_COMPAT=n case (32bit kernel & userspace), > compat_fanotify_mark() isn't used and the fanotify_mark syscall implementation I was not able to find a symbol compat_fanotify_mark. Could you, please, indicate were this coding is. > is used directly. In that case the upper and lower 32 bits of the 64bit mask > parameter is still swapped on big endian machines and thus leads to > fanotify_mark failing with -EINVAL. > > Here is a strace of the same 32bit executable (fanotify01 testcase from LTP): https://github.com/linux-test-project/ltp testcases/kernel/syscalls/fanotify/fanotify01.c I guess. > > On a 64bit kernel it suceeds: > syscall_322(0, 0, 0x3, 0x3, 0x266c8, 0x1) = 0x3 > syscall_323(0x3, 0x1, 0, 0x3b, 0xffffff9c, 0x266c8) = 0 > > On a 32bit kernel it fails: > syscall_322(0, 0, 0x3, 0x3, 0x266c8, 0x1) = 0x3 > syscall_323(0x3, 0x1, 0, 0x3b, 0xffffff9c, 0x266c8) = -1 (errno 22) The syscall numbers are architecture specific. Which architecture did you test on? > > Below is the easiest fix for this problem by simply swapping the upper and > lower 32bit of the 64 bit mask parameter when building a pure 32bit kernel. The problem you report is architecture specific. Is fanotify_user.c really the right place for the correction? Or should the fix be in the "arch" directory? Best regards Heinrich Schuchardt > > But on the other side, using __u64 in a syscall API is IMHO wrong. This may > easily break 32bit kernel builds, esp. on big endian machines. > > The clean solution would probably be to use SYSCALL_DEFINE5() when > building a 64bit-kernel, and SYSCALL_DEFINE6() for fanotify_mark() when > building a pure 32bit kernel, something like this: > > #ifdef CONFIG_64BIT > SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags, > __u64, mask, int, dfd, > const char __user *, pathname) > #else > SYSCALL_DEFINE6(fanotify_mark, int, fanotify_fd, unsigned int, flags, > __u32, mask0, __u32, mask1, int, dfd, > const char __user *, pathname) > #endif > > > Signed-off-by: Helge Deller > To: Eric Paris > Cc: Heinrich Schuchardt > Cc: Heiko Carstens > > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 3fdc8a3..374261c 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -787,6 +787,10 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags, > struct path path; > int ret; > > +#if defined(__BIG_ENDIAN) && !defined(CONFIG_64BIT) > + mask = (mask << 32) | (mask >> 32); > +#endif > + > pr_debug("%s: fanotify_fd=%d flags=%x dfd=%d pathname=%p mask=%llx\n", > __func__, fanotify_fd, flags, dfd, pathname, mask); > >