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 <ptesarik@xxxxxxx> 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 <ptesarik@xxxxxxxx>
> ---
> 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
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
