RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition

[David Laight <David.Laight@ACULAB.COM>]

From: Arnd Bergmann
> Sent: 12 April 2021 12:26
> 
> On Mon, Apr 12, 2021 at 12:54 PM David Laight <David.Laight@aculab.com> wrote:
> > From: David Laight > Sent: 12 April 2021 10:37
> > ...
> > > I'm guessing that compat_pid_t is 16 bits?
> > > So the native 32bit version has an unnamed 2 byte structure pad.
> > > The 'packed' removes this pad from the compat structure.
> > >
> > > AFAICT (apart from mips) the __ARCH_COMPAT_FLOCK_PAD is just
> > > adding an explicit pad for the implicit pad the compiler
> > > would generate because compat_pid_t is 16 bits.
> >
> > I've just looked at the header.
> > compat_pid_t is 32 bits.
> > So Linux must have gained 32bit pids at some earlier time.
> > (Historically Unix pids were 16 bit - even on 32bit systems.)
> >
> > Which makes the explicit pad in 'sparc' rather 'interesting'.
> 
> I saw it was there since the sparc kernel support got merged in
> linux-1.3, possibly copied from an older sunos version.

Which had a 16bit pid when I used it.
So this is a bug in the sparc merge!

The explicit 'short' pad could be removed from the 64bit variant
because there are always 4 bytes of pad after l_pid.
But it does extend the application structure on 32bit sparc so must
remain in the uapi header.
It doesn't need to be in the 'compat' definition.

> > oh - compat_loff_t is only used in a couple of other places.
> > neither care in any way about the alignment.
> > (Provided get_user() doesn't fault on a 8n+4 aligned address.)
> 
> Ah right, I also see that after this series it's only used in to other
> places:  compat_resume_swap_area, which could also lose the
> __packed annotation,

That structure just defines 0 and 8, the structure size doesn't
matter and the offsets are 'passed to' get_user() so byte
accesses aren't performed.

> and in the declaration of
> compat_sys_sendfile64, where it makes no difference.

Which should probably use get_user() rather than copy_from_user().

Although some architectures may need fallback code for
misaligned get_user() ?
Or is there a general 'cop out' that structures passed to the
kernel are required to be correctly aligned.
They should be aligned unless the kernel is 'playing games'
like reading 'struct pollfd' as a 64bit item.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)