Hold on, a follow-up patch is needed to fix --kexec-syscall-auto on s390x. I'm going to resend as a two-patch series.
Petr T On Thu, 12 Mar 2020 21:17:40 +0100 Petr Tesarik <[email protected]> wrote: > The handling of kexec_file_load() error conditions needs some > improvement. > > First, on failure, the system call itself returns -1 and sets > errno. It is wrong to check the return value itself. > > Second, do_kexec_file_load() mixes different types of error > codes (-1, return value of a load method, negative kernel error > number). Let it always return one of the reason codes defined in > kexec/kexec.h. > > Third, the caller of do_kexec_file_load() cannot know what exactly > failed inside that function, so it should not check errno directly. > All it needs to know is whether it makes sense to fall back to the > other syscall. Add an error code for that purpose (EFALLBACK), and > let do_kexec_file_load() decide. > > Fourth, do_kexec_file_load() should not print any error message if > it returns EFALLBACK, because the fallback syscall may succeed > later, and the user is confused whether the command failed, or not. > Move the error message towards the end of main(). > > Signed-off-by: Petr Tesarik <[email protected]> > --- > kexec/kexec.c | 114 > ++++++++++++++++++++++++++++++---------------------------- > kexec/kexec.h | 1 + > 2 files changed, 61 insertions(+), 54 deletions(-) > > diff --git a/kexec/kexec.c b/kexec/kexec.c > index bc6ab3d..33c1b4b 100644 > --- a/kexec/kexec.c > +++ b/kexec/kexec.c > @@ -836,11 +836,21 @@ static int kexec_file_unload(unsigned long > kexec_file_flags) > { > int ret = 0; > > + if (!is_kexec_file_load_implemented()) > + return EFALLBACK; > + > ret = kexec_file_load(-1, -1, 0, NULL, kexec_file_flags); > if (ret != 0) { > - /* The unload failed, print some debugging information */ > - fprintf(stderr, "kexec_file_load(unload) failed\n: %s\n", > - strerror(errno)); > + if (errno == ENOSYS) { > + ret = EFALLBACK; > + } else { > + /* > + * The unload failed, print some debugging > + * information */ > + fprintf(stderr, "kexec_file_load(unload) failed: %s\n", > + strerror(errno)); > + ret = EFAILED; > + } > } > return ret; > } > @@ -1182,15 +1192,13 @@ static int do_kexec_file_load(int fileind, int argc, > char **argv, > info.file_mode = 1; > info.initrd_fd = -1; > > - if (!is_kexec_file_load_implemented()) { > - fprintf(stderr, "syscall kexec_file_load not available.\n"); > - return -ENOSYS; > - } > + if (!is_kexec_file_load_implemented()) > + return EFALLBACK; > > if (argc - fileind <= 0) { > fprintf(stderr, "No kernel specified\n"); > usage(); > - return -1; > + return EFAILED; > } > > kernel = argv[fileind]; > @@ -1199,7 +1207,7 @@ static int do_kexec_file_load(int fileind, int argc, > char **argv, > if (kernel_fd == -1) { > fprintf(stderr, "Failed to open file %s:%s\n", kernel, > strerror(errno)); > - return -1; > + return EFAILED; > } > > /* slurp in the input kernel */ > @@ -1225,7 +1233,7 @@ static int do_kexec_file_load(int fileind, int argc, > char **argv, > if (i == file_types) { > fprintf(stderr, "Cannot determine the file type " "of %s\n", > kernel); > - return -1; > + return EFAILED; > } > > ret = file_type[i].load(argc, argv, kernel_buf, kernel_size, &info); > @@ -1243,9 +1251,43 @@ static int do_kexec_file_load(int fileind, int argc, > char **argv, > > ret = kexec_file_load(kernel_fd, info.initrd_fd, info.command_line_len, > info.command_line, info.kexec_flags); > - if (ret != 0) > - fprintf(stderr, "kexec_file_load failed: %s\n", > - strerror(errno)); > + if (ret != 0) { > + switch (errno) { > + /* > + * Something failed with signature verification. > + * Reject the image. > + */ > + case ELIBBAD: > + case EKEYREJECTED: > + case ENOPKG: > + case ENOKEY: > + case EBADMSG: > + case EMSGSIZE: > + /* Reject by default. */ > + default: > + ret = EFAILED; > + break; > + > + /* Not implemented. */ > + case ENOSYS: > + /* > + * Parsing image or other options failed > + * The image may be invalid or image > + * type may not supported by kernel so > + * retry parsing in kexec-tools. > + */ > + case EINVAL: > + case ENOEXEC: > + /* > + * ENOTSUP can be unsupported image > + * type or unsupported PE signature > + * wrapper type, duh. > + */ > + case ENOTSUP: > + ret = EFALLBACK; > + break; > + } > + } > > close(kernel_fd); > return ret; > @@ -1496,7 +1538,7 @@ int main(int argc, char *argv[]) > if (do_unload) { > if (do_kexec_file_syscall) { > result = kexec_file_unload(kexec_file_flags); > - if ((result == -ENOSYS) && do_kexec_fallback) > + if (result == EFALLBACK && do_kexec_fallback) > do_kexec_file_syscall = 0; > } > if (!do_kexec_file_syscall) > @@ -1506,46 +1548,8 @@ int main(int argc, char *argv[]) > if (do_kexec_file_syscall) { > result = do_kexec_file_load(fileind, argc, argv, > kexec_file_flags); > - if (do_kexec_fallback) switch (result) { > - /* > - * Something failed with signature verification. > - * Reject the image. > - */ > - case -ELIBBAD: > - case -EKEYREJECTED: > - case -ENOPKG: > - case -ENOKEY: > - case -EBADMSG: > - case -EMSGSIZE: > - /* > - * By default reject or do nothing if > - * succeded > - */ > - default: break; > - case -ENOSYS: /* not implemented */ > - /* > - * Parsing image or other options failed > - * The image may be invalid or image > - * type may not supported by kernel so > - * retry parsing in kexec-tools. > - */ > - case -EINVAL: > - case -ENOEXEC: > - /* > - * ENOTSUP can be unsupported image > - * type or unsupported PE signature > - * wrapper type, duh > - * > - * The kernel sometimes wrongly > - * returns ENOTSUPP (524) - ignore > - * that. It is not supposed to be seen > - * by userspace so seeing it is a > - * kernel bug > - */ > - case -ENOTSUP: > - do_kexec_file_syscall = 0; > - break; > - } > + if (result == EFALLBACK && do_kexec_fallback) > + do_kexec_file_syscall = 0; > } > if (!do_kexec_file_syscall) > result = my_load(type, fileind, argc, argv, > @@ -1570,6 +1574,8 @@ int main(int argc, char *argv[]) > if ((result == 0) && do_load_jump_back_helper) { > result = my_load_jump_back_helper(kexec_flags, entry); > } > + if (result == EFALLBACK) > + fputs("syscall kexec_file_load not available.\n", stderr); > > fflush(stdout); > fflush(stderr); > diff --git a/kexec/kexec.h b/kexec/kexec.h > index a97b9ce..28fd129 100644 > --- a/kexec/kexec.h > +++ b/kexec/kexec.h > @@ -63,6 +63,7 @@ > */ > #define EFAILED -1 /* default error code */ > #define ENOCRASHKERNEL -2 /* no memory reserved for crashkernel */ > +#define EFALLBACK -3 /* fallback to kexec_load(2) may work */ > > /* > * This function doesn't actually exist. The idea is that when someone _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
