Re: [PATCH] Documentation: syscalls: add a note about ABI-agnostic types

[Christian Brauner]

On Wed, Apr 14, 2021 at 08:14:22AM +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 13 Apr 2021 21:40:20 -0700
> Yury Norov <yury.norov@xxxxxxxxx> escreveu:
> 
> > Ping?
> > 
> > On Fri, Apr 09, 2021 at 01:43:04PM -0700, Yury Norov wrote:
> > > Recently added memfd_secret() syscall had a flags parameter passed
> > > as unsigned long, which requires creation of compat entry for it.
> > > It was possible to change the type of flags to unsigned int and so
> > > avoid bothering with compat layer.
> > > 
> > > https://www.spinics.net/lists/linux-mm/msg251550.html
> > > 
> > > Documentation/process/adding-syscalls.rst doesn't point clearly about
> > > preference of ABI-agnostic types. This patch adds such notification.
> > > 
> > > Signed-off-by: Yury Norov <yury.norov@xxxxxxxxx>
> > > ---
> > >  Documentation/process/adding-syscalls.rst | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/process/adding-syscalls.rst b/Documentation/process/adding-syscalls.rst
> > > index 9af35f4ec728..46add16edf14 100644
> > > --- a/Documentation/process/adding-syscalls.rst
> > > +++ b/Documentation/process/adding-syscalls.rst
> > > @@ -172,6 +172,13 @@ arguments (i.e. parameter 1, 3, 5), to allow use of contiguous pairs of 32-bit
> > >  registers.  (This concern does not apply if the arguments are part of a
> > >  structure that's passed in by pointer.)
> > >  
> > > +Whenever possible, try to use ABI-agnostic types for passing parameters to
> > > +a syscall in order to avoid creating compat entry for it. Linux supports two
> > > +ABI models - ILP32 and LP64. 
> 
> > > + The types like ``void *``, ``long``, ``size_t``,
> > > +``off_t`` have different size in those ABIs;
> 
> In the case of pointers, the best is to use __u64. The pointer can then
> be read on Kernelspace with something like this:
> 
> 	static inline void __user *media_get_uptr(__u64 arg)
> 	{
> 		return (void __user *)(uintptr_t)arg;
> 	}
> 
> 
> > > types like ``char`` and  ``int``
> > > +have the same size and don't require a compat layer support. For flags, it's
> > > +always better to use ``unsigned int``.
> > > +
> 
> I don't think this is true for all compilers on userspace, as the C
> standard doesn't define how many bits an int/unsigned int has. 
> So, even if this is today's reality, things may change in the future.
> 
> For instance, I remember we had to replace "int" and "enum" by "__u32" 
> and "long" by "__u64" at the media uAPI in the past, when we start
> seeing x86_64 Kernels with 32-bits userspace and when cameras started 
> being supported on arm32.
> 
> We did have some real bugs with "enum", as, on that time, some
> compilers (gcc, I guess) were optimizing them to have less than
> 32 bits on certain architectures, when it fits.

Fwiw, Aleksa and I have written extended syscall documentation
documenting the agreement that we came to in a dedicated session with a
wide range of kernel folks during Linux Plumbers last year. We simply
never had time to actually send this series but fwiw here it is. It also
mentions the use of correct types. If people feel it's worth it I can
send as a proper series: