RE: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load
From: David Laight <hidden>
Date: 2021-05-18 20:47:57
Also in:
kexec, linux-arch, linux-arm-kernel, lkml
From: Arnd Bergmann
quoted hunk ↗ jump to hunk
Sent: 17 May 2021 21:34 The compat version of sys_kexec_load() uses compat_alloc_user_space to convert the user-provided arguments into the native format. Move the conversion into the regular implementation with an in_compat_syscall() check to simplify it and avoid the compat_alloc_user_space() call. compat_sys_kexec_load() now behaves the same as sys_kexec_load(). Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- include/linux/kexec.h | 2 - kernel/kexec.c | 95 +++++++++++++++++++------------------------ 2 files changed, 42 insertions(+), 55 deletions(-)diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 0c994ae37729..f61e310d7a85 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h@@ -88,14 +88,12 @@ struct kexec_segment { size_t memsz; }; -#ifdef CONFIG_COMPAT struct compat_kexec_segment { compat_uptr_t buf; compat_size_t bufsz; compat_ulong_t mem; /* User space sees this as a (void *) ... */ compat_size_t memsz; }; -#endif #ifdef CONFIG_KEXEC_FILE struct purgatory_info {diff --git a/kernel/kexec.c b/kernel/kexec.c index c82c6c06f051..6618b1d9f00b 100644 --- a/kernel/kexec.c +++ b/kernel/kexec.c@@ -19,21 +19,46 @@ #include "kexec_internal.h" +static int copy_user_compat_segment_list(struct kimage *image, + unsigned long nr_segments, + void __user *segments) +{ + struct compat_kexec_segment __user *cs = segments; + struct compat_kexec_segment segment; + int i; + + for (i = 0; i < nr_segments; i++) { + if (copy_from_user(&segment, &cs[i], sizeof(segment))) + return -EFAULT;
How many segments are there? The multiple copy_from_user() will be slow.
+
+ image->segment[i] = (struct kexec_segment) {
+ .buf = compat_ptr(segment.buf),
+ .bufsz = segment.bufsz,
+ .mem = segment.mem,
+ .memsz = segment.memsz,
+ };
+ }
+
+ return 0;
+}
+
+
static int copy_user_segment_list(struct kimage *image,
unsigned long nr_segments,
struct kexec_segment __user *segments)
{
- int ret;
size_t segment_bytes;
/* Read in the segments */
image->nr_segments = nr_segments;
segment_bytes = nr_segments * sizeof(*segments);Should there be a bound check on nr_segments? I can't see one in the code in this patch.
- ret = copy_from_user(image->segment, segments, segment_bytes); - if (ret) - ret = -EFAULT; + if (in_compat_syscall()) + return copy_user_compat_segment_list(image, nr_segments, segments); - return ret; + if (copy_from_user(image->segment, segments, segment_bytes)) + return -EFAULT; + + return 0;
An alternate sequence (which Eric will like even less!) is to do a single copy_from_user() for the entire compat size array into the 'normal' buffer and then do a reverse order conversion of each array entry from 'compat' to '64 bit'. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)