Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Jeff Layton <jlayton at redhat dot com>]

On Thu, 2016-08-18 at 15:24 +0200, Cyril Hrubis wrote:
> Hi!
> > 
> > diff --git a/fs/fcntl.c b/fs/fcntl.c
> > index 350a2c8cfd28..71704aa11170 100644
> > --- a/fs/fcntl.c
> > +++ b/fs/fcntl.c
> > @@ -270,6 +270,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> > > >  	/* 32-bit arches must use fcntl64() */
> > > >  	case F_OFD_GETLK:
> >  #endif
> > > > +	case F_OFD_GETLK32:
> > > >  	case F_GETLK:
> > > >  		err = fcntl_getlk(filp, cmd, (struct flock __user *) arg);
> > > >  		break;
> > @@ -278,7 +279,8 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> > > >  	case F_OFD_SETLK:
> > > >  	case F_OFD_SETLKW:
> >  #endif
> > > > -		/* Fallthrough */
> > > > +	case F_OFD_SETLK32:
> > > > +	case F_OFD_SETLKW32:
> > > >  	case F_SETLK:
> > > >  	case F_SETLKW:
> > > >  		err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg);
> 
> Shouldn't we do #if BITS_PER_LONG == 32 around the newly added cases?
> 
> Since otherwise fcntl() with cmd F_OFD_SETLK32 would expect 64bit off_t
> on 64 bit kernel. It will probably never be used that way, but I find it
> quite confusing.
> 
> The rest looks good to me.
> 

No, 64 bit machines still need these for the compat syscall case.
Consider someone running a 32-bit, non-LFS binary on a 64-bit host.

Unfortunately, the way this has changed over the decades is just really
hard to follow. Eventually we ought to do a cleanup of this code to
make it simpler, but I'd really like this patch to be applicable to
stable kernels, so I think we ought to wait on that until later.

-- 
Jeff Layton <jlayton@redhat.com>