RE: [PATCH v4 2/4] net: socket: rework SIOC?IFMAP ioctls

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

From: Arnd Bergmann
> Sent: 24 November 2020 15:18
> 
> SIOCGIFMAP and SIOCSIFMAP currently require compat_alloc_user_space()
> and copy_in_user() for compat mode.
> 
> Move the compat handling into the location where the structures are
> actually used, to avoid using those interfaces and get a clearer
> implementation.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> changes in v3:
>  - complete rewrite
...
>  include/linux/compat.h | 18 ++++++------
>  net/core/dev_ioctl.c   | 64 +++++++++++++++++++++++++++++++++---------
>  net/socket.c           | 39 ++-----------------------
>  3 files changed, 62 insertions(+), 59 deletions(-)
> 
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 08dbd34bb7a5..47496c5eb5eb 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -96,6 +96,15 @@ struct compat_iovec {
>  	compat_size_t	iov_len;
>  };
> 
> +struct compat_ifmap {
> +	compat_ulong_t mem_start;
> +	compat_ulong_t mem_end;
> +	unsigned short base_addr;
> +	unsigned char irq;
> +	unsigned char dma;
> +	unsigned char port;
> +};

Isn't the only difference the number of pad bytes at the end?
If you don't copy these (in or out) then the compat version
isn't special at all.
Not copying the pad in or out would ensure you don't leak
kernel stack to userspace.
OTOH you may want to write the padding zero.

So a CT_ASSERT(offsetof (struct ifmap, port) == offsetof (struct compat_ifmap, port))
would suffice.

Maybe a CT_ASSERT_EQ_OFFSET(struct ifmap, struct compat_ifmap, port);
Would make the code easier to read.
Although you might want the version that adds an offset

	David

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