RE: [PATCH v4 2/4] net: socket: rework SIOC?IFMAP ioctls
From: David Laight <hidden>
Date: 2020-11-24 16:14:02
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
...
quoted hunk ↗ jump to hunk
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)