[systemd-devel] [PATCH] Fix broken syscall(__NR_fanotify_mark... on 32bit mips.

[Eric Paris]

On Wed, 2011-04-20 at 12:16 +0200, Aurelien Jarno wrote:
> On Wed, Apr 20, 2011 at 03:22:08AM +0200, Lennart Poettering wrote:
> > On Wed, 20.04.11 09:15, fykcee1 at gmail.com (fykcee1 at gmail.com) wrote:
> > 
> > > 
> > > 2011/4/20 Lennart Poettering <lennart at poettering.net>
> > > >
> > > > On Thu, 14.04.11 17:34, fykcee1 at gmail.com (fykcee1 at gmail.com) wrote:
> > > >
> > > > > diff --git a/src/missing.h b/src/missing.h
> > > > > index 35e209f..b367831 100644
> > > > > --- a/src/missing.h
> > > > > +++ b/src/missing.h
> > > > > @@ -125,7 +125,12 @@ static inline int fanotify_init(unsigned int flags, unsigned int event_f_flags)
> > > > >
> > > > >  static inline int fanotify_mark(int fanotify_fd, unsigned int flags, uint64_t mask,
> > > > >                                  int dfd, const char *pathname) {
> > > > > -        return syscall(__NR_fanotify_mark, fanotify_fd, flags, mask, dfd, pathname);
> > > > > +        if (sizeof(void *) == 4)
> > > > > +                return syscall(__NR_fanotify_mark, fanotify_fd, flags,
> > > > > +                               *((uint32_t *) &mask), *((uint32_t *) &mask + 1),
> > > > > +                               dfd, pathname);
> > > > > +        else /* 64bit */
> > > > > +                return syscall(__NR_fanotify_mark, fanotify_fd, flags, mask, dfd, pathname);
> > > > >  }
> > > > >
> > > > >  #ifndef BTRFS_IOCTL_MAGIC
> > > >
> > > > Hmm, wouldn't this break x86-32?
> > > It works fine on my asus laptop with a 32bit CPU Pentium-M.
> > > 
> > > We pass two 32bit arguments instead of one 64bit argument. syscall()
> > > doesn't know the prototype, I guess it will only fetch 9 native-width
> > > arguments(1 syscall number + 8 parameters) from the stack. Aurelien,
> > > am I right?
> > 
> > Hmm, I am not sure I understand this? How is it possible that both ways
> > work on x86? There must be quite a difference between passing 6 or 7
> > arguments to syscall.
> 
> syscall(64_bit_arg) actually pass the 64-bit argument in two different
> registers (or on two different 32-bit stack entries). Then the GNU libc
> copies the registers while converting from the userspace ABI to the
> kernel ABI (usually passing more values into registers).
> 
> On a lot of 32-bit cpus which needs alignment, a 64-bit arguments have 
> to be stack aligned, or register aligned. That's why due to the userspace
> ABI, one register or one stack entry has to be empty. This later break
> when this empty register arrives at the kernel level.
> 
> > Or are you suggesting that the current invocation is broken even on x86,
> > just doesn't become visible since only the lower 32 bit of "mask" are
> > used?
> > 
> 
> No, on x86 it just work "by chance", as x86 is very flexible with
> alignement.

When I implemented the kernel interface I remember someone brought up
64-bit alignment and it was suggest I go with the ordering I have.  I
understood that fanotify_fd would be the first 32 bits, flags be the
second and then mask should be 64-bit aligned as the 3rd/4th?

I am pretty arch stupid, and I have no idea at all if it is related, but
I'm looking at kernel commit 5e844b31c2ace282ab8bea630b63e0212d9532d4
which wires up the fanotify syscalls for mips.  I see that it used a u64
for a3 and a4 when these are only going to be 32 values.  Does that
matter?  I also see that some prototype such as sys32_readahead()
explicitly added a pad into the 32 helper.  Should we be doing that?  I
also notice that the naming of arguments seems wrong (although it
shouldn't matter)

SYSCALL_DEFINE6(32_fanotify_mark, int, fanotify_fd, unsigned int, flags,
                u64, a3, u64, a4, int, dfd, const char  __user *, pathname)

I would have thought equivalently:
  fanotify_fd == a0
  flags == a1
  'a3' == a2
  'a4' == a3
  dfd == a4

etc.....

Anyway, no idea if it is related, but I thought I designed the
systemcall with this padding in mind.....

-Eric