Re: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load

On Tue, May 18, 2021 at 4:05 PM Arnd Bergmann [off-list ref] wrote:

quoted

On Tue, May 18, 2021 at 3:41 PM Eric W. Biederman [off-list ref] wrote:

quoted

Arnd Bergmann [off-list ref] writes:

quoted

From: Arnd Bergmann <arnd@arndb.de>KEXEC_ARCH_DEFAULT

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().

Nacked-by: "Eric W. Biederman" [off-list ref]
KEXEC_ARCH_DEFAULT
The patch is wrong.

The logic between the compat entry point and the ordinary entry point
are by necessity different.   This unifies the logic and breaks the compat
entry point.

The fundamentally necessity is that the code being loaded needs to know
which mode the kernel is running in so it can safely transition to the
new kernel.

Given that the two entry points fundamentally need different logic,
and that difference was not preserved and the goal of this patchset
was to unify that which fundamentally needs to be different.  I don't
think this patch series makes any sense for kexec.

Sorry, I'm not following that explanation. Can you clarify what different
modes of the kernel you are referring to here, and how my patch
changes this?

I think I figured it out now myself after comparing the two functions:

--- a/kernel/kexec.c
+++ b/kernel/kexec.c

@@ -269,7 +269,8 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry,

unsigned long, nr_segments,

        /* Verify we are on the appropriate architecture */
        if (((flags & KEXEC_ARCH_MASK) != KEXEC_ARCH) &&
-               ((flags & KEXEC_ARCH_MASK) != KEXEC_ARCH_DEFAULT))
+               (in_compat_syscall() ||
+               ((flags & KEXEC_ARCH_MASK) != KEXEC_ARCH_DEFAULT)))
                return -EINVAL;

        /* Because we write directly to the reserved memory

Not sure if that's the best way of doing it, but it looks like folding this
in restores the current behavior.