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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Nov 2020 16:13:35 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Arnd Bergmann' <arnd@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Arnd Bergmann <arnd@...db.de>
Subject: RE: [PATCH v4 2/4] net: socket: rework SIOC?IFMAP ioctls

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@...db.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)