[PATCH 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[PATCH 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchanek]

When the kernel does not know a syscall number it returns -ENOSYS but
when kexec does not know a syscall number it returns -1. Return -ENOSYS
from kexec as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index cfd837c1b6bb..ab8cff7fe083 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
 
 	if (!is_kexec_file_load_implemented()) {
 		fprintf(stderr, "syscall kexec_file_load not available.\n");
-		return -1;
+		return -ENOSYS;
 	}
 
 	if (argc - fileind <= 0) {
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/5] kexec: do not special-case the -s option

[Michal Suchanek]

It is parsed separately to save a few CPU cycles when setting up other
options but it just complicates the code. So fold it back and set up all
flags both for KEXEC_LOAD and KEXEC_FILE_LOAD

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index ab8cff7fe083..9ea102e1565a 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
 	};
 	static const char short_options[] = KEXEC_ALL_OPT_STR;
 
-	/*
-	 * First check if --use-kexec-file-syscall is set. That changes lot of
-	 * things
-	 */
-	while ((opt = getopt_long(argc, argv, short_options,
-				  options, 0)) != -1) {
-		switch(opt) {
-		case OPT_KEXEC_FILE_SYSCALL:
-			do_kexec_file_syscall = 1;
-			break;
-		}
-	}
-
 	/* Reset getopt for the next pass. */
 	opterr = 1;
 	optind = 1;
@@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
 			do_shutdown = 0;
 			do_sync = 0;
 			do_unload = 1;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_UNLOAD;
+			kexec_file_flags |= KEXEC_FILE_UNLOAD;
 			break;
 		case OPT_EXEC:
 			do_load = 0;
@@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
 			do_exec = 0;
 			do_shutdown = 0;
 			do_sync = 0;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
-			else
-				kexec_flags = KEXEC_ON_CRASH;
-			break;
+			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
+			kexec_flags = KEXEC_ON_CRASH;
 		case OPT_MEM_MIN:
 			mem_min = strtoul(optarg, &endptr, 0);
 			if (*endptr) {
@@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
 			do_reuse_initrd = 1;
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
-			/* We already parsed it. Nothing to do. */
+			do_kexec_file_syscall = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 3/5] kexec: add option to revert -s

[Michal Suchanek]

The undocumented -s option selects KEXEC_FILE_LOAD syscall but there is
no option to select KEXEC_LOAD syscall so add it. It is generally good
idea and in followup patch the default will be changed so it will be
needed to get KEXEC_LOAD functionality.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 3 +++
 kexec/kexec.h | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 9ea102e1565a..a95cfb473d6b 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1368,6 +1368,9 @@ int main(int argc, char *argv[])
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
 			break;
+		case OPT_KEXEC_SYSCALL:
+			do_kexec_file_syscall = 0;
+			break;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 26225d2c002a..9fd0355eacd0 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -219,6 +219,7 @@ extern int file_types;
 #define OPT_TYPE		't'
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
+#define OPT_KEXEC_SYSCALL	'c'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -246,11 +247,12 @@ extern int file_types;
 	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
+	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:psS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

When no option is passed to select one syscall or the other try
KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index a95cfb473d6b..14f56e466a95 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 1;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_KEXEC_SYSCALL:
 			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
@@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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:
+				case -ENOTSUPP:
+					/*
+					 * By default reject or do nothing if
+					 * succeded
+					 */
+				default: break;
+					/*
+					 * 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:
+					do_kexec_file_syscall = 0;
+					break;
+			}
+		}
+		if (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 5/5] kexec: document -s and -c options.

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.8 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kexec/kexec.8 b/kexec/kexec.8
index e0131b4ea827..f3b1556f749e 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -144,6 +144,13 @@ Load the new kernel for use on panic.
 Specify that the new kernel is of this
 .I type.
 .TP
+.BI \-s\ (\-\-kexec-file-syscall)
+Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
+Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used.
+.TP
+.BI \-c\ (\-\-kexec-syscall)
+Specify that the old KEXEC_LOAD syscall should be used exclusively.
+.TP
 .B \-u\ (\-\-unload)
 Unload the current
 .B kexec
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Mon, 26 Feb 2018 13:00:37 +0100
Michal Suchanek <msuchanek@suse.de> wrote:

> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled
> in locked-down mode. Previously users had to select the
> KEXEC_FILE_LOAD syscall with undocumented -s option. However, if they
> did pass the option kexec would fail on architectures that do not
> support it.
> 
> When no option is passed to select one syscall or the other try
> KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..14f56e466a95 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
>+	int do_kexec_fallback = 1;

do_kexec_file_syscall should be also set to 1 for the fallback to be
the default. Or we need the fallback option to set do_kexec_fallback
separately.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> When no option is passed to select one syscall or the other try
> KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..14f56e466a95 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 1;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> @@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +				case -ENOTSUPP:
> +					/*
> +					 * By default reject or do nothing if
> +					 * succeded
> +					 */
> +				default: break;
> +					/*
> +					 * 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:
> +					do_kexec_file_syscall = 0;
> +					break;

Why do we need checking so many errno, I assumed only fallback in case -ENOSYS

> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> When no option is passed to select one syscall or the other try
> KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.

Again, IMHO the default behavior should not be changed..

> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..14f56e466a95 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 1;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> @@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +				case -ENOTSUPP:
> +					/*
> +					 * By default reject or do nothing if
> +					 * succeded
> +					 */
> +				default: break;
> +					/*
> +					 * 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:
> +					do_kexec_file_syscall = 0;
> +					break;
> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri, 2 Mar 2018 17:24:19 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > enabled in locked-down mode. Previously users had to select the
> > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > they did pass the option kexec would fail on architectures that do
> > not support it.
> > 
> > When no option is passed to select one syscall or the other try
> > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.  
> 
> Again, IMHO the default behavior should not be changed..

So adding a new option to enable the fallback would be ok?

Thanks

Michal

> 
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 39 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index a95cfb473d6b..14f56e466a95 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >  	int do_unload = 0;
> >  	int do_reuse_initrd = 0;
> >  	int do_kexec_file_syscall = 0;
> > +	int do_kexec_fallback = 1;
> >  	int do_status = 0;
> >  	void *entry = 0;
> >  	char *type = 0;
> > @@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
> >  			break;
> >  		case OPT_KEXEC_FILE_SYSCALL:
> >  			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_KEXEC_SYSCALL:
> >  			do_kexec_file_syscall = 0;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_STATUS:
> >  			do_status = 1;
> > @@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
> >  		result = k_status(kexec_flags);
> >  	}
> >  	if (do_unload) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result =
> > kexec_file_unload(kexec_file_flags);
> > -		else
> > +			if ((result == -ENOSYS) &&
> > do_kexec_fallback)
> > +				do_kexec_file_syscall = 0;
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = k_unload(kexec_flags);
> >  	}
> >  	if (do_load && (result == 0)) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = do_kexec_file_load(fileind, argc,
> > argv, kexec_file_flags);
> > -		else
> > +			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:
> > +				case -ENOTSUPP:
> > +					/*
> > +					 * By default reject or do
> > nothing if
> > +					 * succeded
> > +					 */
> > +				default: break;
> > +					/*
> > +					 * 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:
> > +					do_kexec_file_syscall = 0;
> > +					break;
> > +			}
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = my_load(type, fileind, argc, argv,
> >  						kexec_flags,
> > entry); }
> > -- 
> > 2.13.6
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec  
> 
> Thanks
> Dave


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Simon Horman]

On Mon, Feb 26, 2018 at 01:00:34PM +0100, Michal Suchanek wrote:
> When the kernel does not know a syscall number it returns -ENOSYS but
> when kexec does not know a syscall number it returns -1. Return -ENOSYS
> from kexec as well.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index cfd837c1b6bb..ab8cff7fe083 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
>  
>  	if (!is_kexec_file_load_implemented()) {
>  		fprintf(stderr, "syscall kexec_file_load not available.\n");
> -		return -1;
> +		return -ENOSYS;

It seems to me that the current with do_kexec_file_load() returning
0 for success and -1 for error. I think any change to that scheme would
need to be more comprehensive than this.

>  	}
>  
>  	if (argc - fileind <= 0) {
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Mon, Feb 26, 2018 at 01:00:35PM +0100, Michal Suchanek wrote:
> It is parsed separately to save a few CPU cycles when setting up other
> options but it just complicates the code. So fold it back and set up all
> flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index ab8cff7fe083..9ea102e1565a 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
>  	};
>  	static const char short_options[] = KEXEC_ALL_OPT_STR;
>  
> -	/*
> -	 * First check if --use-kexec-file-syscall is set. That changes lot of
> -	 * things
> -	 */
> -	while ((opt = getopt_long(argc, argv, short_options,
> -				  options, 0)) != -1) {
> -		switch(opt) {
> -		case OPT_KEXEC_FILE_SYSCALL:
> -			do_kexec_file_syscall = 1;
> -			break;
> -		}
> -	}
> -
>  	/* Reset getopt for the next pass. */
>  	opterr = 1;
>  	optind = 1;
> @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
>  			do_shutdown = 0;
>  			do_sync = 0;
>  			do_unload = 1;
> -			if (do_kexec_file_syscall)
> -				kexec_file_flags |= KEXEC_FILE_UNLOAD;
> +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
>  			break;
>  		case OPT_EXEC:
>  			do_load = 0;
> @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
>  			do_exec = 0;
>  			do_shutdown = 0;
>  			do_sync = 0;
> -			if (do_kexec_file_syscall)
> -				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> -			else
> -				kexec_flags = KEXEC_ON_CRASH;
> -			break;
> +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> +			kexec_flags = KEXEC_ON_CRASH;

This appears to change the logic of options parsing.

>  		case OPT_MEM_MIN:
>  			mem_min = strtoul(optarg, &endptr, 0);
>  			if (*endptr) {
> @@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
>  			do_reuse_initrd = 1;
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
> -			/* We already parsed it. Nothing to do. */
> +			do_kexec_file_syscall = 1;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Simon Horman]

On Mon, Feb 26, 2018 at 01:00:37PM +0100, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> When no option is passed to select one syscall or the other try
> KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.

Has there been a release of kexec-tools with things arranged they
way they currently are?

Can we document this behaviour and the related undocumented options?

> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..14f56e466a95 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 1;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> @@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +				case -ENOTSUPP:
> +					/*
> +					 * By default reject or do nothing if
> +					 * succeded
> +					 */
> +				default: break;
> +					/*
> +					 * 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:
> +					do_kexec_file_syscall = 0;
> +					break;
> +			}
> +		}
> +		if (!do_kexec_file_syscall)

How about:

	if (do_kexec_file_syscall) {
		result = ...;
		/* Comment */
		if (do_kexec_fallback &&
		    (result == -EINVAL || result == -ENOEXEC))
			do_kexec_file_syscall = 0;
	}
	if (!do_kexec_file_syscall)
		result = ...;



>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Simon Horman]

On Fri, Mar 02, 2018 at 01:32:52PM +0100, Michal Suchánek wrote:
> On Fri, 2 Mar 2018 17:24:19 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > enabled in locked-down mode. Previously users had to select the
> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > they did pass the option kexec would fail on architectures that do
> > > not support it.
> > > 
> > > When no option is passed to select one syscall or the other try
> > > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.  
> > 
> > Again, IMHO the default behavior should not be changed..
> 
> So adding a new option to enable the fallback would be ok?

I am also wary of changing the default behaviour and
I think a new fallback option would be better.

Dave?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri, 2 Mar 2018 13:46:10 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Fri, Mar 02, 2018 at 01:32:52PM +0100, Michal Suchánek wrote:
> > On Fri, 2 Mar 2018 17:24:19 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >   
> > > On 02/26/18 at 01:00pm, Michal Suchanek wrote:  
> > > > Not all architectures implement KEXEC_FILE_LOAD. However, on
> > > > some archiectures KEXEC_FILE_LOAD is required when secure boot
> > > > is enabled in locked-down mode. Previously users had to select
> > > > the KEXEC_FILE_LOAD syscall with undocumented -s option.
> > > > However, if they did pass the option kexec would fail on
> > > > architectures that do not support it.
> > > > 
> > > > When no option is passed to select one syscall or the other try
> > > > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not
> > > > suported.    
> > > 
> > > Again, IMHO the default behavior should not be changed..  
> > 
> > So adding a new option to enable the fallback would be ok?  
> 
> I am also wary of changing the default behaviour and
> I think a new fallback option would be better.
> 
> Dave?

Ok, I will resend with a separate fallback option.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchanek]

When the kernel does not know a syscall number it returns -ENOSYS but
when kexec does not know a syscall number it returns -1. Return -ENOSYS
from kexec as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index cfd837c1b6bb..ab8cff7fe083 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
 
 	if (!is_kexec_file_load_implemented()) {
 		fprintf(stderr, "syscall kexec_file_load not available.\n");
-		return -1;
+		return -ENOSYS;
 	}
 
 	if (argc - fileind <= 0) {
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 2/5] kexec: do not special-case the -s option

[Michal Suchanek]

It is parsed separately to save a few CPU cycles when setting up other
options but it just complicates the code. So fold it back and set up all
flags both for KEXEC_LOAD and KEXEC_FILE_LOAD

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index ab8cff7fe083..9ea102e1565a 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
 	};
 	static const char short_options[] = KEXEC_ALL_OPT_STR;
 
-	/*
-	 * First check if --use-kexec-file-syscall is set. That changes lot of
-	 * things
-	 */
-	while ((opt = getopt_long(argc, argv, short_options,
-				  options, 0)) != -1) {
-		switch(opt) {
-		case OPT_KEXEC_FILE_SYSCALL:
-			do_kexec_file_syscall = 1;
-			break;
-		}
-	}
-
 	/* Reset getopt for the next pass. */
 	opterr = 1;
 	optind = 1;
@@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
 			do_shutdown = 0;
 			do_sync = 0;
 			do_unload = 1;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_UNLOAD;
+			kexec_file_flags |= KEXEC_FILE_UNLOAD;
 			break;
 		case OPT_EXEC:
 			do_load = 0;
@@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
 			do_exec = 0;
 			do_shutdown = 0;
 			do_sync = 0;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
-			else
-				kexec_flags = KEXEC_ON_CRASH;
-			break;
+			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
+			kexec_flags = KEXEC_ON_CRASH;
 		case OPT_MEM_MIN:
 			mem_min = strtoul(optarg, &endptr, 0);
 			if (*endptr) {
@@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
 			do_reuse_initrd = 1;
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
-			/* We already parsed it. Nothing to do. */
+			do_kexec_file_syscall = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 3/5] kexec: add option to revert -s

[Michal Suchanek]

The undocumented -s option selects KEXEC_FILE_LOAD syscall but there is
no option to select KEXEC_LOAD syscall so add it. It is generally good
idea and in followup patch the default will be changed so it will be
needed to get KEXEC_LOAD functionality.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 3 +++
 kexec/kexec.h | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 9ea102e1565a..a95cfb473d6b 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1368,6 +1368,9 @@ int main(int argc, char *argv[])
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
 			break;
+		case OPT_KEXEC_SYSCALL:
+			do_kexec_file_syscall = 0;
+			break;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 26225d2c002a..9fd0355eacd0 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -219,6 +219,7 @@ extern int file_types;
 #define OPT_TYPE		't'
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
+#define OPT_KEXEC_SYSCALL	'c'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -246,11 +247,12 @@ extern int file_types;
 	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
+	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:psS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

When no option is passed to select one syscall or the other try
KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
 kexec/kexec.h |  4 +++-
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index a95cfb473d6b..c7aaf8ac147f 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 0;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_KEXEC_SYSCALL:
 			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
 			break;
+		case OPT_KEXEC_SYSCALL_AUTO:
+			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 1;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
@@ -1442,16 +1448,53 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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;
+					/*
+					 * 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:
+					 /*
+					  * ENOTSUPP can be unsupported image
+					  * type or unsupported PE signature
+					  * wrapper type, duh
+					  */
+				case -ENOTSUP:
+					do_kexec_file_syscall = 0;
+					break;
+			}
+		}
+		if (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 9fd0355eacd0..d445fbe3e486 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -220,6 +220,7 @@ extern int file_types;
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
 #define OPT_KEXEC_SYSCALL	'c'
+#define OPT_KEXEC_SYSCALL_AUTO	'a'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -248,11 +249,12 @@ extern int file_types;
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
 	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
+	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3 5/5] kexec: document -s, -c and -a options.

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.8 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/kexec/kexec.8 b/kexec/kexec.8
index e0131b4ea827..b3543db3f413 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -144,6 +144,21 @@ Load the new kernel for use on panic.
 Specify that the new kernel is of this
 .I type.
 .TP
+.BI \-s\ (\-\-kexec-file-syscall)
+Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
+.TP
+.BI \-c\ (\-\-kexec-syscall)
+Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).
+.TP
+.BI \-a\ (\-\-kexec-syscall-auto)
+Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not supported
+fall back to the old KEXEC_LOAD interface.
+
+There is no one single interface that always works. KEXEC_FILE_LOAD is required
+on systems that use locked-down secure boot to verify the kernel signature.
+KEXEC_LOAD is required for some kernel image formats and on architectures that
+do not support KEXEC_FILE_LOAD.
+.TP
 .B \-u\ (\-\-unload)
 Unload the current
 .B kexec
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/5] kexec: do not special-case the -s option

[Michal Suchánek]

On Fri, 2 Mar 2018 13:36:16 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Mon, Feb 26, 2018 at 01:00:35PM +0100, Michal Suchanek wrote:
> > It is parsed separately to save a few CPU cycles when setting up
> > other options but it just complicates the code. So fold it back and
> > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 25 ++++---------------------
> >  1 file changed, 4 insertions(+), 21 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index ab8cff7fe083..9ea102e1565a 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> >  	};
> >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> >  
> > -	/*
> > -	 * First check if --use-kexec-file-syscall is set. That
> > changes lot of
> > -	 * things
> > -	 */
> > -	while ((opt = getopt_long(argc, argv, short_options,
> > -				  options, 0)) != -1) {
> > -		switch(opt) {
> > -		case OPT_KEXEC_FILE_SYSCALL:
> > -			do_kexec_file_syscall = 1;
> > -			break;
> > -		}
> > -	}
> > -
> >  	/* Reset getopt for the next pass. */
> >  	opterr = 1;
> >  	optind = 1;
> > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> >  			do_unload = 1;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_UNLOAD;
> > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> >  			break;
> >  		case OPT_EXEC:
> >  			do_load = 0;
> > @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> >  			do_exec = 0;
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_ON_CRASH;
> > -			else
> > -				kexec_flags = KEXEC_ON_CRASH;
> > -			break;
> > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > +			kexec_flags = KEXEC_ON_CRASH;  
> 
> This appears to change the logic of options parsing.
> 

Care to share how exactly is the logic changed? I do not see it. To me
it looks like we have separate flags for KEXEC_LOAD and KEXEC_FILE_LOAD
so setting up both does not change anything. Whatever syscall is used
it has the flags set up.

In the existing logic we decide which syscall to do beforehand and set
up only flags for that syscall but that does not save much CPU cycles
and makes the code more complex and fragile. 

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchánek]

On Fri, 2 Mar 2018 13:34:45 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Mon, Feb 26, 2018 at 01:00:34PM +0100, Michal Suchanek wrote:
> > When the kernel does not know a syscall number it returns -ENOSYS
> > but when kexec does not know a syscall number it returns -1. Return
> > -ENOSYS from kexec as well.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index cfd837c1b6bb..ab8cff7fe083 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind,
> > int argc, char **argv, 
> >  	if (!is_kexec_file_load_implemented()) {
> >  		fprintf(stderr, "syscall kexec_file_load not
> > available.\n");
> > -		return -1;
> > +		return -ENOSYS;  
> 
> It seems to me that the current with do_kexec_file_load() returning
> 0 for success and -1 for error. I think any change to that scheme
> would need to be more comprehensive than this.

It looks to me that the return value from the actual syscall is passed
through as the return value of kexec, at least for kexec_file_load. So
this only fixes that one occasion when -ENOSYS can originate from kexec
itself. Sure, there are more return values that could be made more
sensible.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri,  2 Mar 2018 14:33:02 +0100
Michal Suchanek <msuchanek@suse.de> wrote:

> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled
> in locked-down mode. Previously users had to select the
> KEXEC_FILE_LOAD syscall with undocumented -s option. However, if they
> did pass the option kexec would fail on architectures that do not
> support it.
> 
> When no option is passed to select one syscall or the other try
> KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.

I forgot to update the description. meh

> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 51
> +++++++++++++++++++++++++++++++++++++++++++++++---- kexec/kexec.h |
> 4 +++- 2 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..c7aaf8ac147f 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 0;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
> +		case OPT_KEXEC_SYSCALL_AUTO:
> +			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 1;
>  		case OPT_STATUS:
>  			do_status = 1;
>  			break;
> @@ -1442,16 +1448,53 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc,
> argv, kexec_file_flags);
> -		else
> +			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;
> +					/*
> +					 * 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:
> +					 /*
> +					  * ENOTSUPP can be
> unsupported image
> +					  * type or unsupported PE
> signature
> +					  * wrapper type, duh
> +					  */
> +				case -ENOTSUP:
> +					do_kexec_file_syscall = 0;
> +					break;
> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 9fd0355eacd0..d445fbe3e486 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -220,6 +220,7 @@ extern int file_types;
>  #define OPT_PANIC		'p'
>  #define OPT_KEXEC_FILE_SYSCALL	's'
>  #define OPT_KEXEC_SYSCALL	'c'
> +#define OPT_KEXEC_SYSCALL_AUTO	'a'
>  #define OPT_STATUS		'S'
>  #define OPT_MEM_MIN             256
>  #define OPT_MEM_MAX             257
> @@ -248,11 +249,12 @@ extern int file_types;
>  	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
>  	{ "kexec-file-syscall",	0, 0,
> OPT_KEXEC_FILE_SYSCALL }, \ { "kexec-syscall",	0, 0,
> OPT_KEXEC_SYSCALL }, \
> +	{ "kexec-syscall-auto",	0, 0,
> OPT_KEXEC_SYSCALL_AUTO }, \ { "debug",		0, 0,
> OPT_DEBUG }, \ { "status",		0, 0, OPT_STATUS }, \
>  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  
> -#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
> +#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
>  
>  extern void dbgprint_mem_range(const char *prefix, struct
> memory_range *mr, int nr_mr); extern void die(const char *fmt, ...)


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/02/18 at 01:46pm, Simon Horman wrote:
> On Fri, Mar 02, 2018 at 01:32:52PM +0100, Michal Suchánek wrote:
> > On Fri, 2 Mar 2018 17:24:19 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> > 
> > > On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> > > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > > enabled in locked-down mode. Previously users had to select the
> > > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > > they did pass the option kexec would fail on architectures that do
> > > > not support it.
> > > > 
> > > > When no option is passed to select one syscall or the other try
> > > > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.  
> > > 
> > > Again, IMHO the default behavior should not be changed..
> > 
> > So adding a new option to enable the fallback would be ok?
> 
> I am also wary of changing the default behaviour and
> I think a new fallback option would be better.
> 
> Dave?

Simon, I have same feeling, and a new option looks good to me as well

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Fri, Mar 02, 2018 at 02:38:22PM +0100, Michal Suchánek wrote:
> On Fri, 2 Mar 2018 13:36:16 +0100
> Simon Horman <horms@verge.net.au> wrote:
> 
> > On Mon, Feb 26, 2018 at 01:00:35PM +0100, Michal Suchanek wrote:
> > > It is parsed separately to save a few CPU cycles when setting up
> > > other options but it just complicates the code. So fold it back and
> > > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  kexec/kexec.c | 25 ++++---------------------
> > >  1 file changed, 4 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index ab8cff7fe083..9ea102e1565a 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> > >  	};
> > >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> > >  
> > > -	/*
> > > -	 * First check if --use-kexec-file-syscall is set. That
> > > changes lot of
> > > -	 * things
> > > -	 */
> > > -	while ((opt = getopt_long(argc, argv, short_options,
> > > -				  options, 0)) != -1) {
> > > -		switch(opt) {
> > > -		case OPT_KEXEC_FILE_SYSCALL:
> > > -			do_kexec_file_syscall = 1;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > >  	/* Reset getopt for the next pass. */
> > >  	opterr = 1;
> > >  	optind = 1;
> > > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > >  			do_unload = 1;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_UNLOAD;
> > > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> > >  			break;
> > >  		case OPT_EXEC:
> > >  			do_load = 0;
> > > @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> > >  			do_exec = 0;
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_ON_CRASH;
> > > -			else
> > > -				kexec_flags = KEXEC_ON_CRASH;
> > > -			break;
> > > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > > +			kexec_flags = KEXEC_ON_CRASH;  
> > 
> > This appears to change the logic of options parsing.
> > 
> 
> Care to share how exactly is the logic changed? I do not see it. To me
> it looks like we have separate flags for KEXEC_LOAD and KEXEC_FILE_LOAD
> so setting up both does not change anything. Whatever syscall is used
> it has the flags set up.
> 
> In the existing logic we decide which syscall to do beforehand and set
> up only flags for that syscall but that does not save much CPU cycles
> and makes the code more complex and fragile. 

Thanks, the logic change that I was referring to is that kexec_file_flags
and kexec_flags are both updated regardless of the value of
do_kexec_file_syscall, which was not the case prior to this patch. If that
has no effect on the overall execution of the program then this is of
course fine but that was not at all obvious to me.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

So add an -a option that tries KEXEC_FILE_LOAD and when it is not
supported tries KEXEC_LOAD.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
 kexec/kexec.h |  4 +++-
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index a95cfb473d6b..c7aaf8ac147f 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 0;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_KEXEC_SYSCALL:
 			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
 			break;
+		case OPT_KEXEC_SYSCALL_AUTO:
+			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 1;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
@@ -1442,16 +1448,53 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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;
+					/*
+					 * 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:
+					 /*
+					  * ENOTSUPP can be unsupported image
+					  * type or unsupported PE signature
+					  * wrapper type, duh
+					  */
+				case -ENOTSUP:
+					do_kexec_file_syscall = 0;
+					break;
+			}
+		}
+		if (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 9fd0355eacd0..d445fbe3e486 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -220,6 +220,7 @@ extern int file_types;
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
 #define OPT_KEXEC_SYSCALL	'c'
+#define OPT_KEXEC_SYSCALL_AUTO	'a'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -248,11 +249,12 @@ extern int file_types;
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
 	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
+	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri, 2 Mar 2018 17:17:06 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 02/26/18 at 01:00pm, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > enabled in locked-down mode. Previously users had to select the
> > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > they did pass the option kexec would fail on architectures that do
> > not support it.
> > 
> > When no option is passed to select one syscall or the other try
> > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 43 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 39 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index a95cfb473d6b..14f56e466a95 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >  	int do_unload = 0;
> >  	int do_reuse_initrd = 0;
> >  	int do_kexec_file_syscall = 0;
> > +	int do_kexec_fallback = 1;
> >  	int do_status = 0;
> >  	void *entry = 0;
> >  	char *type = 0;
> > @@ -1367,9 +1368,11 @@ int main(int argc, char *argv[])
> >  			break;
> >  		case OPT_KEXEC_FILE_SYSCALL:
> >  			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_KEXEC_SYSCALL:
> >  			do_kexec_file_syscall = 0;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_STATUS:
> >  			do_status = 1;
> > @@ -1442,16 +1445,48 @@ int main(int argc, char *argv[])
> >  		result = k_status(kexec_flags);
> >  	}
> >  	if (do_unload) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result =
> > kexec_file_unload(kexec_file_flags);
> > -		else
> > +			if ((result == -ENOSYS) &&
> > do_kexec_fallback)
> > +				do_kexec_file_syscall = 0;
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = k_unload(kexec_flags);
> >  	}
> >  	if (do_load && (result == 0)) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = do_kexec_file_load(fileind, argc,
> > argv, kexec_file_flags);
> > -		else
> > +			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:

The above do not need to be listed. These are the return values
expected when checking signature fails. This lists is for documentation
purposes only.

> > +				case -ENOTSUPP:
> > +					/*
> > +					 * By default reject or do
> > nothing if
> > +					 * succeded
> > +					 */
> > +				default: break;
> > +					/*
> > +					 * 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:
> > +					do_kexec_file_syscall = 0;
> > +					break;  
> 
> Why do we need checking so many errno, 

Linux returns these when it does not understand the image format.
Because some formats are not supported by KEXEC_LOAD_FILE it makes sense
to try parsing the image in kexec-tools.

> I assumed only fallback in
> case -ENOSYS

which is not even listed.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchanek]

When the kernel does not know a syscall number it returns -ENOSYS but
when kexec does not know a syscall number it returns -1. Return -ENOSYS
from kexec as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index cfd837c1b6bb..ab8cff7fe083 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
 
 	if (!is_kexec_file_load_implemented()) {
 		fprintf(stderr, "syscall kexec_file_load not available.\n");
-		return -1;
+		return -ENOSYS;
 	}
 
 	if (argc - fileind <= 0) {
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 2/5] kexec: do not special-case the -s option

[Michal Suchanek]

It is parsed separately to save a few CPU cycles when setting up other
options but it just complicates the code. So fold it back and set up all
flags both for KEXEC_LOAD and KEXEC_FILE_LOAD

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index ab8cff7fe083..9ea102e1565a 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
 	};
 	static const char short_options[] = KEXEC_ALL_OPT_STR;
 
-	/*
-	 * First check if --use-kexec-file-syscall is set. That changes lot of
-	 * things
-	 */
-	while ((opt = getopt_long(argc, argv, short_options,
-				  options, 0)) != -1) {
-		switch(opt) {
-		case OPT_KEXEC_FILE_SYSCALL:
-			do_kexec_file_syscall = 1;
-			break;
-		}
-	}
-
 	/* Reset getopt for the next pass. */
 	opterr = 1;
 	optind = 1;
@@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
 			do_shutdown = 0;
 			do_sync = 0;
 			do_unload = 1;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_UNLOAD;
+			kexec_file_flags |= KEXEC_FILE_UNLOAD;
 			break;
 		case OPT_EXEC:
 			do_load = 0;
@@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
 			do_exec = 0;
 			do_shutdown = 0;
 			do_sync = 0;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
-			else
-				kexec_flags = KEXEC_ON_CRASH;
-			break;
+			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
+			kexec_flags = KEXEC_ON_CRASH;
 		case OPT_MEM_MIN:
 			mem_min = strtoul(optarg, &endptr, 0);
 			if (*endptr) {
@@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
 			do_reuse_initrd = 1;
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
-			/* We already parsed it. Nothing to do. */
+			do_kexec_file_syscall = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 3/5] kexec: add option to revert -s

[Michal Suchanek]

The undocumented -s option selects KEXEC_FILE_LOAD syscall but there is
no option to select KEXEC_LOAD syscall so add it. It is generally good
idea and in followup patch the default will be changed so it will be
needed to get KEXEC_LOAD functionality.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 3 +++
 kexec/kexec.h | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 9ea102e1565a..a95cfb473d6b 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1368,6 +1368,9 @@ int main(int argc, char *argv[])
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
 			break;
+		case OPT_KEXEC_SYSCALL:
+			do_kexec_file_syscall = 0;
+			break;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 26225d2c002a..9fd0355eacd0 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -219,6 +219,7 @@ extern int file_types;
 #define OPT_TYPE		't'
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
+#define OPT_KEXEC_SYSCALL	'c'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -246,11 +247,12 @@ extern int file_types;
 	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
+	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:psS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

So add an -a option that tries KEXEC_FILE_LOAD and when it is not
supported tries KEXEC_LOAD.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: instead of changing the deafult add extra option
v4: actually check -ENOSYS as well
---
 kexec/kexec.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
 kexec/kexec.h |  4 +++-
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index a95cfb473d6b..5c5aee344b41 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 0;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_KEXEC_SYSCALL:
 			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
 			break;
+		case OPT_KEXEC_SYSCALL_AUTO:
+			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 1;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
@@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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:
+					 /*
+					  * ENOTSUPP can be unsupported image
+					  * type or unsupported PE signature
+					  * wrapper type, duh
+					  */
+				case -ENOTSUP:
+					do_kexec_file_syscall = 0;
+					break;
+			}
+		}
+		if (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 9fd0355eacd0..d445fbe3e486 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -220,6 +220,7 @@ extern int file_types;
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
 #define OPT_KEXEC_SYSCALL	'c'
+#define OPT_KEXEC_SYSCALL_AUTO	'a'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -248,11 +249,12 @@ extern int file_types;
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
 	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
+	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 5/5] kexec: document -s, -c and -a options.

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.8 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/kexec/kexec.8 b/kexec/kexec.8
index e0131b4ea827..b3543db3f413 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -144,6 +144,21 @@ Load the new kernel for use on panic.
 Specify that the new kernel is of this
 .I type.
 .TP
+.BI \-s\ (\-\-kexec-file-syscall)
+Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
+.TP
+.BI \-c\ (\-\-kexec-syscall)
+Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).
+.TP
+.BI \-a\ (\-\-kexec-syscall-auto)
+Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not supported
+fall back to the old KEXEC_LOAD interface.
+
+There is no one single interface that always works. KEXEC_FILE_LOAD is required
+on systems that use locked-down secure boot to verify the kernel signature.
+KEXEC_LOAD is required for some kernel image formats and on architectures that
+do not support KEXEC_FILE_LOAD.
+.TP
 .B \-u\ (\-\-unload)
 Unload the current
 .B kexec
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Tony Jones]

On 03/06/2018 05:15 AM, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> supported tries KEXEC_LOAD.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>

David, Simon.

Is there any feedback on v4 of this patchset?  Adding a new option for fallback rather than changing the existing behavior was the v3 feedback on March 2nd.

Cheers!

Tony

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/5] kexec: fallback to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri, 2 Mar 2018 13:44:58 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Mon, Feb 26, 2018 at 01:00:37PM +0100, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > enabled in locked-down mode. Previously users had to select the
> > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > they did pass the option kexec would fail on architectures that do
> > not support it.
> > 
> > When no option is passed to select one syscall or the other try
> > KEXEC_FILE_LOAD and fall back to KEXEC_LOAD when not suported.  
> 
> Has there been a release of kexec-tools with things arranged they
> way they currently are?

$ git tag --contains
046d1755d2bd723a11a180c265e61a884990712e 
v2.0.10
v2.0.10-rc1
v2.0.11
v2.0.11-rc1
v2.0.12
v2.0.12-rc1
v2.0.13
v2.0.13-rc1
v2.0.14
v2.0.14-rc1
v2.0.15
v2.0.15-rc1
v2.0.16-rc1
v2.0.8
v2.0.8-rc1
v2.0.9
v2.0.9-rc1

> 
> Can we document this behaviour and the related undocumented options?

It's done in a followup patch.

Thanks

Michal


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> supported tries KEXEC_LOAD.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v3: instead of changing the deafult add extra option
> v4: actually check -ENOSYS as well
> ---
>  kexec/kexec.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  kexec/kexec.h |  4 +++-
>  2 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..5c5aee344b41 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 0;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
> +		case OPT_KEXEC_SYSCALL_AUTO:
> +			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 1;

need a break here

>  		case OPT_STATUS:
>  			do_status = 1;
>  			break;
> @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +					 /*
> +					  * ENOTSUPP can be unsupported image
> +					  * type or unsupported PE signature
> +					  * wrapper type, duh
> +					  */
> +				case -ENOTSUP:
> +					do_kexec_file_syscall = 0;
> +					break;

It looks to me it is enough only checking -ENOSYS maybe also -ENOTSUPP and
then set do_kexec_file_syscall = 0;

EINVAL and ENOEXEC are real errors, I do not understand why still 
fallback.  Also thos signature verification errors are not needed
in this code as well.

> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 9fd0355eacd0..d445fbe3e486 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -220,6 +220,7 @@ extern int file_types;
>  #define OPT_PANIC		'p'
>  #define OPT_KEXEC_FILE_SYSCALL	's'
>  #define OPT_KEXEC_SYSCALL	'c'
> +#define OPT_KEXEC_SYSCALL_AUTO	'a'
>  #define OPT_STATUS		'S'
>  #define OPT_MEM_MIN             256
>  #define OPT_MEM_MAX             257
> @@ -248,11 +249,12 @@ extern int file_types;
>  	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
>  	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
>  	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
> +	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
>  	{ "debug",		0, 0, OPT_DEBUG }, \
>  	{ "status",		0, 0, OPT_STATUS }, \
>  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  
> -#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
> +#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
>  
>  extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
>  extern void die(const char *fmt, ...)
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> supported tries KEXEC_LOAD.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v3: instead of changing the deafult add extra option
> v4: actually check -ENOSYS as well
> ---
>  kexec/kexec.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  kexec/kexec.h |  4 +++-
>  2 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a95cfb473d6b..5c5aee344b41 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 0;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
>  			break;
>  		case OPT_KEXEC_SYSCALL:
>  			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
>  			break;
> +		case OPT_KEXEC_SYSCALL_AUTO:
> +			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 1;
>  		case OPT_STATUS:
>  			do_status = 1;
>  			break;
> @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +					 /*
> +					  * ENOTSUPP can be unsupported image
> +					  * type or unsupported PE signature
> +					  * wrapper type, duh
> +					  */
> +				case -ENOTSUP:

Shouldn't this be -ENOTSUPP ?

> +					do_kexec_file_syscall = 0;
> +					break;
> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 9fd0355eacd0..d445fbe3e486 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -220,6 +220,7 @@ extern int file_types;
>  #define OPT_PANIC		'p'
>  #define OPT_KEXEC_FILE_SYSCALL	's'
>  #define OPT_KEXEC_SYSCALL	'c'
> +#define OPT_KEXEC_SYSCALL_AUTO	'a'
>  #define OPT_STATUS		'S'
>  #define OPT_MEM_MIN             256
>  #define OPT_MEM_MAX             257
> @@ -248,11 +249,12 @@ extern int file_types;
>  	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
>  	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
>  	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
> +	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
>  	{ "debug",		0, 0, OPT_DEBUG }, \
>  	{ "status",		0, 0, OPT_STATUS }, \
>  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  
> -#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
> +#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
>  
>  extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
>  extern void die(const char *fmt, ...)
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Dave Young]

On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.8 | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/kexec/kexec.8 b/kexec/kexec.8
> index e0131b4ea827..b3543db3f413 100644
> --- a/kexec/kexec.8
> +++ b/kexec/kexec.8
> @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
>  Specify that the new kernel is of this
>  .I type.
>  .TP
> +.BI \-s\ (\-\-kexec-file-syscall)
> +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.

Maybe better to be simple like below:
"Use kexec_file_load syscall to load the new kernel."


> +.TP
> +.BI \-c\ (\-\-kexec-syscall)
> +Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).

similarly:
"Use kexec_load syscall to load the new kernel."

> +.TP
> +.BI \-a\ (\-\-kexec-syscall-auto)
> +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not supported
> +fall back to the old KEXEC_LOAD interface.
> +
> +There is no one single interface that always works. KEXEC_FILE_LOAD is required
> +on systems that use locked-down secure boot to verify the kernel signature.
> +KEXEC_LOAD is required for some kernel image formats and on architectures that
> +do not support KEXEC_FILE_LOAD.

It seems not good to say kexec_file_load is simpler and newer.  Also it
is not a must for Secure Boot and locked down kernel only. So it would
be better to just simplify and use the first paragraph:

"Try kexec_file_load syscall first and if it is not supported fall back
to the kexec_load syscall"

> +.TP
>  .B \-u\ (\-\-unload)
>  Unload the current
>  .B kexec
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Dave Young]

On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.8 | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/kexec/kexec.8 b/kexec/kexec.8
> index e0131b4ea827..b3543db3f413 100644
> --- a/kexec/kexec.8
> +++ b/kexec/kexec.8
> @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
>  Specify that the new kernel is of this
>  .I type.
>  .TP
> +.BI \-s\ (\-\-kexec-file-syscall)
> +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
> +.TP
> +.BI \-c\ (\-\-kexec-syscall)
> +Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).
> +.TP
> +.BI \-a\ (\-\-kexec-syscall-auto)
> +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not supported
> +fall back to the old KEXEC_LOAD interface.
> +
> +There is no one single interface that always works. KEXEC_FILE_LOAD is required
> +on systems that use locked-down secure boot to verify the kernel signature.
> +KEXEC_LOAD is required for some kernel image formats and on architectures that
> +do not support KEXEC_FILE_LOAD.
> +.TP
>  .B \-u\ (\-\-unload)
>  Unload the current
>  .B kexec
> -- 
> 2.13.6
> 

Actually while I replied this mail, I tried 'kexec -c -a' system
immediately rebooted.  Also as I noted in patch 4 a 'break' is missing
so does it work on your side?

I think it need more testing.

Another question I have is not sure if '-c' is a good option letter, but
I do not have suggestion as well..

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/13/18 at 10:30am, Tony Jones wrote:
> On 03/06/2018 05:15 AM, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> > locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> > syscall with undocumented -s option. However, if they did pass the
> > option kexec would fail on architectures that do not support it.
> > 
> > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > supported tries KEXEC_LOAD.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> 
> David, Simon.
> 
> Is there any feedback on v4 of this patchset?  Adding a new option for fallback rather than changing the existing behavior was the v3 feedback on March 2nd.
> 

Tony, sure, I just replied with several comments, but still need Simon
to review in case I may miss something.

> Cheers!
> 
> Tony

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Wed, 14 Mar 2018 11:22:40 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > enabled in locked-down mode. Previously users had to select the
> > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > they did pass the option kexec would fail on architectures that do
> > not support it.
> > 
> > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > supported tries KEXEC_LOAD.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v3: instead of changing the deafult add extra option
> > v4: actually check -ENOSYS as well
> > ---
> >  kexec/kexec.c | 52
> > ++++++++++++++++++++++++++++++++++++++++++++++++---- kexec/kexec.h
> > |  4 +++- 2 files changed, 51 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index a95cfb473d6b..5c5aee344b41 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >  	int do_unload = 0;
> >  	int do_reuse_initrd = 0;
> >  	int do_kexec_file_syscall = 0;
> > +	int do_kexec_fallback = 0;
> >  	int do_status = 0;
> >  	void *entry = 0;
> >  	char *type = 0;
> > @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
> >  			break;
> >  		case OPT_KEXEC_FILE_SYSCALL:
> >  			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_KEXEC_SYSCALL:
> >  			do_kexec_file_syscall = 0;
> > +			do_kexec_fallback = 0;
> >  			break;
> > +		case OPT_KEXEC_SYSCALL_AUTO:
> > +			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 1;
> >  		case OPT_STATUS:
> >  			do_status = 1;
> >  			break;
> > @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
> >  		result = k_status(kexec_flags);
> >  	}
> >  	if (do_unload) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result =
> > kexec_file_unload(kexec_file_flags);
> > -		else
> > +			if ((result == -ENOSYS) &&
> > do_kexec_fallback)
> > +				do_kexec_file_syscall = 0;
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = k_unload(kexec_flags);
> >  	}
> >  	if (do_load && (result == 0)) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = do_kexec_file_load(fileind, argc,
> > argv, kexec_file_flags);
> > -		else
> > +			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:
> > +					 /*
> > +					  * ENOTSUPP can be
> > unsupported image
> > +					  * type or unsupported PE
> > signature
> > +					  * wrapper type, duh
> > +					  */
> > +				case -ENOTSUP:  
> 
> Shouldn't this be -ENOTSUPP ?

No, ENOTSUP and EOPNOTSUPP is defined. The latter is for socket
operations.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Michal Suchánek]

On Wed, 14 Mar 2018 11:41:30 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.8 | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > index e0131b4ea827..b3543db3f413 100644
> > --- a/kexec/kexec.8
> > +++ b/kexec/kexec.8
> > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> >  Specify that the new kernel is of this
> >  .I type.
> >  .TP
> > +.BI \-s\ (\-\-kexec-file-syscall)
> > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > exclusively.  
> 
> Maybe better to be simple like below:
> "Use kexec_file_load syscall to load the new kernel."
> 
> 
> > +.TP
> > +.BI \-c\ (\-\-kexec-syscall)
> > +Specify that the old KEXEC_LOAD syscall should be used exclusively
> > (the default).  
> 
> similarly:
> "Use kexec_load syscall to load the new kernel."
> 
> > +.TP
> > +.BI \-a\ (\-\-kexec-syscall-auto)
> > +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not
> > supported +fall back to the old KEXEC_LOAD interface.
> > +
> > +There is no one single interface that always works.
> > KEXEC_FILE_LOAD is required +on systems that use locked-down secure
> > boot to verify the kernel signature. +KEXEC_LOAD is required for
> > some kernel image formats and on architectures that +do not support
> > KEXEC_FILE_LOAD.  
> 
> It seems not good to say kexec_file_load is simpler and newer.  Also
> it is not a must for Secure Boot and locked down kernel only. So it
> would be better to just simplify and use the first paragraph:
> 
> "Try kexec_file_load syscall first and if it is not supported fall
> back to the kexec_load syscall"

There was a request for explanation so just the first paragraph will
not do. What is it required for other than secure boot?

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/14/18 at 08:23am, Michal Suchánek wrote:
> On Wed, 14 Mar 2018 11:22:40 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > enabled in locked-down mode. Previously users had to select the
> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > they did pass the option kexec would fail on architectures that do
> > > not support it.
> > > 
> > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > > supported tries KEXEC_LOAD.
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v3: instead of changing the deafult add extra option
> > > v4: actually check -ENOSYS as well
> > > ---
> > >  kexec/kexec.c | 52
> > > ++++++++++++++++++++++++++++++++++++++++++++++++---- kexec/kexec.h
> > > |  4 +++- 2 files changed, 51 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index a95cfb473d6b..5c5aee344b41 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> > >  	int do_unload = 0;
> > >  	int do_reuse_initrd = 0;
> > >  	int do_kexec_file_syscall = 0;
> > > +	int do_kexec_fallback = 0;
> > >  	int do_status = 0;
> > >  	void *entry = 0;
> > >  	char *type = 0;
> > > @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
> > >  			break;
> > >  		case OPT_KEXEC_FILE_SYSCALL:
> > >  			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 0;
> > >  			break;
> > >  		case OPT_KEXEC_SYSCALL:
> > >  			do_kexec_file_syscall = 0;
> > > +			do_kexec_fallback = 0;
> > >  			break;
> > > +		case OPT_KEXEC_SYSCALL_AUTO:
> > > +			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 1;
> > >  		case OPT_STATUS:
> > >  			do_status = 1;
> > >  			break;
> > > @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
> > >  		result = k_status(kexec_flags);
> > >  	}
> > >  	if (do_unload) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result =
> > > kexec_file_unload(kexec_file_flags);
> > > -		else
> > > +			if ((result == -ENOSYS) &&
> > > do_kexec_fallback)
> > > +				do_kexec_file_syscall = 0;
> > > +		}
> > > +		if (!do_kexec_file_syscall)
> > >  			result = k_unload(kexec_flags);
> > >  	}
> > >  	if (do_load && (result == 0)) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result = do_kexec_file_load(fileind, argc,
> > > argv, kexec_file_flags);
> > > -		else
> > > +			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:
> > > +					 /*
> > > +					  * ENOTSUPP can be
> > > unsupported image
> > > +					  * type or unsupported PE
> > > signature
> > > +					  * wrapper type, duh
> > > +					  */
> > > +				case -ENOTSUP:  
> > 
> > Shouldn't this be -ENOTSUPP ?
> 
> No, ENOTSUP and EOPNOTSUPP is defined. The latter is for socket
> operations.

Your code comment "ENOTSUPP can be unsupported image", but your
code use ENOTSUP,   ENOTSUP != ENOTSUPP here.

> 
> Thanks
> 
> Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Dave Young]

On 03/14/18 at 08:25am, Michal Suchánek wrote:
> On Wed, 14 Mar 2018 11:41:30 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  kexec/kexec.8 | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > > index e0131b4ea827..b3543db3f413 100644
> > > --- a/kexec/kexec.8
> > > +++ b/kexec/kexec.8
> > > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> > >  Specify that the new kernel is of this
> > >  .I type.
> > >  .TP
> > > +.BI \-s\ (\-\-kexec-file-syscall)
> > > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > > exclusively.  
> > 
> > Maybe better to be simple like below:
> > "Use kexec_file_load syscall to load the new kernel."
> > 
> > 
> > > +.TP
> > > +.BI \-c\ (\-\-kexec-syscall)
> > > +Specify that the old KEXEC_LOAD syscall should be used exclusively
> > > (the default).  
> > 
> > similarly:
> > "Use kexec_load syscall to load the new kernel."
> > 
> > > +.TP
> > > +.BI \-a\ (\-\-kexec-syscall-auto)
> > > +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not
> > > supported +fall back to the old KEXEC_LOAD interface.
> > > +
> > > +There is no one single interface that always works.
> > > KEXEC_FILE_LOAD is required +on systems that use locked-down secure
> > > boot to verify the kernel signature. +KEXEC_LOAD is required for
> > > some kernel image formats and on architectures that +do not support
> > > KEXEC_FILE_LOAD.  
> > 
> > It seems not good to say kexec_file_load is simpler and newer.  Also
> > it is not a must for Secure Boot and locked down kernel only. So it
> > would be better to just simplify and use the first paragraph:
> > 
> > "Try kexec_file_load syscall first and if it is not supported fall
> > back to the kexec_load syscall"
> 
> There was a request for explanation so just the first paragraph will
> not do. What is it required for other than secure boot?

People can use kexec -s to load a signed kernel but not necessary to
boot with Secure Boot enabled.

There is no Secure Boot in powerpc, arm64 now.

> 
> Thanks
> 
> Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Tue, Mar 06, 2018 at 02:15:53PM +0100, Michal Suchanek wrote:
> It is parsed separately to save a few CPU cycles when setting up other
> options but it just complicates the code. So fold it back and set up all
> flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index ab8cff7fe083..9ea102e1565a 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
>  	};
>  	static const char short_options[] = KEXEC_ALL_OPT_STR;
>  
> -	/*
> -	 * First check if --use-kexec-file-syscall is set. That changes lot of
> -	 * things
> -	 */
> -	while ((opt = getopt_long(argc, argv, short_options,
> -				  options, 0)) != -1) {
> -		switch(opt) {
> -		case OPT_KEXEC_FILE_SYSCALL:
> -			do_kexec_file_syscall = 1;
> -			break;
> -		}
> -	}
> -
>  	/* Reset getopt for the next pass. */
>  	opterr = 1;
>  	optind = 1;
> @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
>  			do_shutdown = 0;
>  			do_sync = 0;
>  			do_unload = 1;
> -			if (do_kexec_file_syscall)
> -				kexec_file_flags |= KEXEC_FILE_UNLOAD;
> +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
>  			break;
>  		case OPT_EXEC:
>  			do_load = 0;
 @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])

 The existing code has the following above the context shown in the patch:

			do_load = 1;


>  			do_exec = 0;
>  			do_shutdown = 0;
>  			do_sync = 0;
> -			if (do_kexec_file_syscall)
> -				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> -			else
> -				kexec_flags = KEXEC_ON_CRASH;
> -			break;
> +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> +			kexec_flags = KEXEC_ON_CRASH;

kexec_flags is now set regardless of the value of do_kexec_file_syscall,
which was not the case prior to this patch. That seems to affect the
following which appears later in the function. Is that ok?


        if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
            !is_crashkernel_mem_reserved()) {
                die("Memory for crashkernel is not reserved\n"
                    "Please reserve memory by passing"
                    "\"crashkernel=X@Y\" parameter to kernel\n"
                    "Then try to loading kdump kernel\n");
        }

	...

        if ((result == 0) && do_load_jump_back_helper) {
                result = my_load_jump_back_helper(kexec_flags, entry);
        }

>  		case OPT_MEM_MIN:
>  			mem_min = strtoul(optarg, &endptr, 0);
>  			if (*endptr) {
> @@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
>  			do_reuse_initrd = 1;
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
> -			/* We already parsed it. Nothing to do. */
> +			do_kexec_file_syscall = 1;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Wed, 14 Mar 2018 11:21:59 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > enabled in locked-down mode. Previously users had to select the
> > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > they did pass the option kexec would fail on architectures that do
> > not support it.
> > 
> > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > supported tries KEXEC_LOAD.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v3: instead of changing the deafult add extra option
> > v4: actually check -ENOSYS as well
> > ---
> >  kexec/kexec.c | 52
> > ++++++++++++++++++++++++++++++++++++++++++++++++---- kexec/kexec.h
> > |  4 +++- 2 files changed, 51 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index a95cfb473d6b..5c5aee344b41 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >  	int do_unload = 0;
> >  	int do_reuse_initrd = 0;
> >  	int do_kexec_file_syscall = 0;
> > +	int do_kexec_fallback = 0;
> >  	int do_status = 0;
> >  	void *entry = 0;
> >  	char *type = 0;
> > @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
> >  			break;
> >  		case OPT_KEXEC_FILE_SYSCALL:
> >  			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 0;
> >  			break;
> >  		case OPT_KEXEC_SYSCALL:
> >  			do_kexec_file_syscall = 0;
> > +			do_kexec_fallback = 0;
> >  			break;
> > +		case OPT_KEXEC_SYSCALL_AUTO:
> > +			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 1;  
> 
> need a break here

Indeed

> 
> >  		case OPT_STATUS:
> >  			do_status = 1;
> >  			break;
> > @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
> >  		result = k_status(kexec_flags);
> >  	}
> >  	if (do_unload) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result =
> > kexec_file_unload(kexec_file_flags);
> > -		else
> > +			if ((result == -ENOSYS) &&
> > do_kexec_fallback)
> > +				do_kexec_file_syscall = 0;
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = k_unload(kexec_flags);
> >  	}
> >  	if (do_load && (result == 0)) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = do_kexec_file_load(fileind, argc,
> > argv, kexec_file_flags);
> > -		else
> > +			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:
> > +					 /*
> > +					  * ENOTSUPP can be
> > unsupported image
> > +					  * type or unsupported PE
> > signature
> > +					  * wrapper type, duh
> > +					  */
> > +				case -ENOTSUP:
> > +					do_kexec_file_syscall = 0;
> > +					break;  
> 
> It looks to me it is enough only checking -ENOSYS maybe also
> -ENOTSUPP and then set do_kexec_file_syscall = 0;
> 
> EINVAL and ENOEXEC are real errors, I do not understand why still 
> fallback.  

If you pass an image type that the kernel does not understand (eg.
multiboot or uImage) then the kernel will return a real error because
it does not understand the image. However, kexec-tools should still be
able to load it, automatically. That's what the -auto stands for.

> Also thos signature verification errors are not needed
> in this code as well.

Yes, they are not needed. They are here so it's obvious which errors
are signature verification errors.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] kexec: do not special-case the -s option

[Michal Suchánek]

On Thu, 15 Mar 2018 11:38:30 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Tue, Mar 06, 2018 at 02:15:53PM +0100, Michal Suchanek wrote:
> > It is parsed separately to save a few CPU cycles when setting up
> > other options but it just complicates the code. So fold it back and
> > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 25 ++++---------------------
> >  1 file changed, 4 insertions(+), 21 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index ab8cff7fe083..9ea102e1565a 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> >  	};
> >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> >  
> > -	/*
> > -	 * First check if --use-kexec-file-syscall is set. That
> > changes lot of
> > -	 * things
> > -	 */
> > -	while ((opt = getopt_long(argc, argv, short_options,
> > -				  options, 0)) != -1) {
> > -		switch(opt) {
> > -		case OPT_KEXEC_FILE_SYSCALL:
> > -			do_kexec_file_syscall = 1;
> > -			break;
> > -		}
> > -	}
> > -
> >  	/* Reset getopt for the next pass. */
> >  	opterr = 1;
> >  	optind = 1;
> > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> >  			do_unload = 1;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_UNLOAD;
> > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> >  			break;
> >  		case OPT_EXEC:
> >  			do_load = 0;  
>  @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> 
>  The existing code has the following above the context shown in the
> patch:
> 
> 			do_load = 1;
> 
> 
> >  			do_exec = 0;
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_ON_CRASH;
> > -			else
> > -				kexec_flags = KEXEC_ON_CRASH;
> > -			break;
> > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > +			kexec_flags = KEXEC_ON_CRASH;  
> 
> kexec_flags is now set regardless of the value of
> do_kexec_file_syscall, which was not the case prior to this patch.
> That seems to affect the following which appears later in the
> function. Is that ok?
> 
> 
>         if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
>             !is_crashkernel_mem_reserved()) {
>                 die("Memory for crashkernel is not reserved\n"
>                     "Please reserve memory by passing"
>                     "\"crashkernel=X@Y\" parameter to kernel\n"
>                     "Then try to loading kdump kernel\n");
>         }

Do you not need memory for kexec -s? This looks broken to start with.
> 
> 	...
> 
>         if ((result == 0) && do_load_jump_back_helper) {

And yes, this should not be allowed with -s

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Michal Suchánek]

On Wed, 14 Mar 2018 11:43:55 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.8 | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > index e0131b4ea827..b3543db3f413 100644
> > --- a/kexec/kexec.8
> > +++ b/kexec/kexec.8
> > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> >  Specify that the new kernel is of this
> >  .I type.
> >  .TP
> > +.BI \-s\ (\-\-kexec-file-syscall)
> > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > exclusively. +.TP
> > +.BI \-c\ (\-\-kexec-syscall)
> > +Specify that the old KEXEC_LOAD syscall should be used exclusively
> > (the default). +.TP
> > +.BI \-a\ (\-\-kexec-syscall-auto)
> > +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is not
> > supported +fall back to the old KEXEC_LOAD interface.
> > +
> > +There is no one single interface that always works.
> > KEXEC_FILE_LOAD is required +on systems that use locked-down secure
> > boot to verify the kernel signature. +KEXEC_LOAD is required for
> > some kernel image formats and on architectures that +do not support
> > KEXEC_FILE_LOAD. +.TP
> >  .B \-u\ (\-\-unload)
> >  Unload the current
> >  .B kexec
> > -- 
> > 2.13.6
> >   
> 
> Actually while I replied this mail, I tried 'kexec -c -a' system
> immediately rebooted.  Also as I noted in patch 4 a 'break' is missing
> so does it work on your side?

Right, the original single-patch code had a bug which is now fixed but
new one crept in. Since we still don't agree how it *should* behave it
is too early to say it deviates from expectation.

If we agree what it should do but it turns out to be broken it can be
fixed after the fact, too.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Michal Suchánek]

On Wed, 14 Mar 2018 15:50:31 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/14/18 at 08:25am, Michal Suchánek wrote:
> > On Wed, 14 Mar 2018 11:41:30 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >   
> > > On 03/06/18 at 02:15pm, Michal Suchanek wrote:  
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > >  kexec/kexec.8 | 15 +++++++++++++++
> > > >  1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > > > index e0131b4ea827..b3543db3f413 100644
> > > > --- a/kexec/kexec.8
> > > > +++ b/kexec/kexec.8
> > > > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> > > >  Specify that the new kernel is of this
> > > >  .I type.
> > > >  .TP
> > > > +.BI \-s\ (\-\-kexec-file-syscall)
> > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > > > exclusively.    
> > > 
> > > Maybe better to be simple like below:
> > > "Use kexec_file_load syscall to load the new kernel."
> > > 
> > >   
> > > > +.TP
> > > > +.BI \-c\ (\-\-kexec-syscall)
> > > > +Specify that the old KEXEC_LOAD syscall should be used
> > > > exclusively (the default).    
> > > 
> > > similarly:
> > > "Use kexec_load syscall to load the new kernel."
> > >   
> > > > +.TP
> > > > +.BI \-a\ (\-\-kexec-syscall-auto)
> > > > +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is
> > > > not supported +fall back to the old KEXEC_LOAD interface.
> > > > +
> > > > +There is no one single interface that always works.
> > > > KEXEC_FILE_LOAD is required +on systems that use locked-down
> > > > secure boot to verify the kernel signature. +KEXEC_LOAD is
> > > > required for some kernel image formats and on architectures
> > > > that +do not support KEXEC_FILE_LOAD.    
> > > 
> > > It seems not good to say kexec_file_load is simpler and newer.
> > > Also it is not a must for Secure Boot and locked down kernel
> > > only. So it would be better to just simplify and use the first
> > > paragraph:
> > > 
> > > "Try kexec_file_load syscall first and if it is not supported fall
> > > back to the kexec_load syscall"  
> > 
> > There was a request for explanation so just the first paragraph will
> > not do. What is it required for other than secure boot?  
> 
> People can use kexec -s to load a signed kernel but not necessary to
> boot with Secure Boot enabled.

Is booting signed kernel without -s not supported? If so I would
consider it kexec-tools bug. And it should documented then as well I
guess.

> 
> There is no Secure Boot in powerpc, arm64 now.

Is there not yet? Anyway, the intent is to support it which is probably
the reason we have the syscall in the first place.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Dave Young]

On 03/15/18 at 12:06pm, Michal Suchánek wrote:
> On Wed, 14 Mar 2018 11:21:59 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/06/18 at 02:15pm, Michal Suchanek wrote:
> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > enabled in locked-down mode. Previously users had to select the
> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > they did pass the option kexec would fail on architectures that do
> > > not support it.
> > > 
> > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > > supported tries KEXEC_LOAD.
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v3: instead of changing the deafult add extra option
> > > v4: actually check -ENOSYS as well
> > > ---
> > >  kexec/kexec.c | 52
> > > ++++++++++++++++++++++++++++++++++++++++++++++++---- kexec/kexec.h
> > > |  4 +++- 2 files changed, 51 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index a95cfb473d6b..5c5aee344b41 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> > >  	int do_unload = 0;
> > >  	int do_reuse_initrd = 0;
> > >  	int do_kexec_file_syscall = 0;
> > > +	int do_kexec_fallback = 0;
> > >  	int do_status = 0;
> > >  	void *entry = 0;
> > >  	char *type = 0;
> > > @@ -1367,10 +1368,15 @@ int main(int argc, char *argv[])
> > >  			break;
> > >  		case OPT_KEXEC_FILE_SYSCALL:
> > >  			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 0;
> > >  			break;
> > >  		case OPT_KEXEC_SYSCALL:
> > >  			do_kexec_file_syscall = 0;
> > > +			do_kexec_fallback = 0;
> > >  			break;
> > > +		case OPT_KEXEC_SYSCALL_AUTO:
> > > +			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 1;  
> > 
> > need a break here
> 
> Indeed
> 
> > 
> > >  		case OPT_STATUS:
> > >  			do_status = 1;
> > >  			break;
> > > @@ -1442,16 +1448,54 @@ int main(int argc, char *argv[])
> > >  		result = k_status(kexec_flags);
> > >  	}
> > >  	if (do_unload) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result =
> > > kexec_file_unload(kexec_file_flags);
> > > -		else
> > > +			if ((result == -ENOSYS) &&
> > > do_kexec_fallback)
> > > +				do_kexec_file_syscall = 0;
> > > +		}
> > > +		if (!do_kexec_file_syscall)
> > >  			result = k_unload(kexec_flags);
> > >  	}
> > >  	if (do_load && (result == 0)) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result = do_kexec_file_load(fileind, argc,
> > > argv, kexec_file_flags);
> > > -		else
> > > +			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:
> > > +					 /*
> > > +					  * ENOTSUPP can be
> > > unsupported image
> > > +					  * type or unsupported PE
> > > signature
> > > +					  * wrapper type, duh
> > > +					  */
> > > +				case -ENOTSUP:
> > > +					do_kexec_file_syscall = 0;
> > > +					break;  
> > 
> > It looks to me it is enough only checking -ENOSYS maybe also
> > -ENOTSUPP and then set do_kexec_file_syscall = 0;
> > 
> > EINVAL and ENOEXEC are real errors, I do not understand why still 
> > fallback.  
> 
> If you pass an image type that the kernel does not understand (eg.
> multiboot or uImage) then the kernel will return a real error because
> it does not understand the image. However, kexec-tools should still be
> able to load it, automatically. That's what the -auto stands for.

This semes over engineering, the initial purpose is to fallback when
kexec_file_load is not supported, so I would suggest not to do more
than that.

> 
> > Also thos signature verification errors are not needed
> > in this code as well.
> 
> Yes, they are not needed. They are here so it's obvious which errors
> are signature verification errors.
> 
> Thanks
> 
> Michal

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Dave Young]

On 03/15/18 at 12:44pm, Michal Suchánek wrote:
> On Wed, 14 Mar 2018 15:50:31 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/14/18 at 08:25am, Michal Suchánek wrote:
> > > On Wed, 14 Mar 2018 11:41:30 +0800
> > > Dave Young <dyoung@redhat.com> wrote:
> > >   
> > > > On 03/06/18 at 02:15pm, Michal Suchanek wrote:  
> > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > > ---
> > > > >  kexec/kexec.8 | 15 +++++++++++++++
> > > > >  1 file changed, 15 insertions(+)
> > > > > 
> > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > > > > index e0131b4ea827..b3543db3f413 100644
> > > > > --- a/kexec/kexec.8
> > > > > +++ b/kexec/kexec.8
> > > > > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> > > > >  Specify that the new kernel is of this
> > > > >  .I type.
> > > > >  .TP
> > > > > +.BI \-s\ (\-\-kexec-file-syscall)
> > > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > > > > exclusively.    
> > > > 
> > > > Maybe better to be simple like below:
> > > > "Use kexec_file_load syscall to load the new kernel."
> > > > 
> > > >   
> > > > > +.TP
> > > > > +.BI \-c\ (\-\-kexec-syscall)
> > > > > +Specify that the old KEXEC_LOAD syscall should be used
> > > > > exclusively (the default).    
> > > > 
> > > > similarly:
> > > > "Use kexec_load syscall to load the new kernel."
> > > >   
> > > > > +.TP
> > > > > +.BI \-a\ (\-\-kexec-syscall-auto)
> > > > > +Try the new simpler KEXEC_FILE_LOAD syscall first and if it is
> > > > > not supported +fall back to the old KEXEC_LOAD interface.
> > > > > +
> > > > > +There is no one single interface that always works.
> > > > > KEXEC_FILE_LOAD is required +on systems that use locked-down
> > > > > secure boot to verify the kernel signature. +KEXEC_LOAD is
> > > > > required for some kernel image formats and on architectures
> > > > > that +do not support KEXEC_FILE_LOAD.    
> > > > 
> > > > It seems not good to say kexec_file_load is simpler and newer.
> > > > Also it is not a must for Secure Boot and locked down kernel
> > > > only. So it would be better to just simplify and use the first
> > > > paragraph:
> > > > 
> > > > "Try kexec_file_load syscall first and if it is not supported fall
> > > > back to the kexec_load syscall"  
> > > 
> > > There was a request for explanation so just the first paragraph will
> > > not do. What is it required for other than secure boot?  
> > 
> > People can use kexec -s to load a signed kernel but not necessary to
> > boot with Secure Boot enabled.
> 
> Is booting signed kernel without -s not supported? If so I would
> consider it kexec-tools bug. And it should documented then as well I
> guess.

I'm not sure I understand the question.  In kernel we splitted kexec and
kexec_file they can be enabled as kernel config options separately.  If
one want to a secured kexec (not UEFI Secure Boot, only signed kernel
loading) then one can only enable CONFIG_KEXEC_FILE but disable
CONFIG_KEXEC.  In this case without '-s' load will fail.  But if one
enabled both CONFIG_KEXEC_FILE and CONFIG_KEXEC then kexec load without
'-s' still works.

> 
> > 
> > There is no Secure Boot in powerpc, arm64 now.
> 
> Is there not yet? Anyway, the intent is to support it which is probably
> the reason we have the syscall in the first place.

Secure Boot is UEFI only, AFAIK powerpc does not have UEFI,  arm64 has
UEFI but I do not see Secure Boot.

Also powerpc version kexec_file_load does not have signature
verification.

> 
> Thanks
> 
> Michal

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Thu, Mar 15, 2018 at 12:13:18PM +0100, Michal Suchánek wrote:
> On Thu, 15 Mar 2018 11:38:30 +0100
> Simon Horman <horms@verge.net.au> wrote:
> 
> > On Tue, Mar 06, 2018 at 02:15:53PM +0100, Michal Suchanek wrote:
> > > It is parsed separately to save a few CPU cycles when setting up
> > > other options but it just complicates the code. So fold it back and
> > > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  kexec/kexec.c | 25 ++++---------------------
> > >  1 file changed, 4 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index ab8cff7fe083..9ea102e1565a 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> > >  	};
> > >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> > >  
> > > -	/*
> > > -	 * First check if --use-kexec-file-syscall is set. That
> > > changes lot of
> > > -	 * things
> > > -	 */
> > > -	while ((opt = getopt_long(argc, argv, short_options,
> > > -				  options, 0)) != -1) {
> > > -		switch(opt) {
> > > -		case OPT_KEXEC_FILE_SYSCALL:
> > > -			do_kexec_file_syscall = 1;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > >  	/* Reset getopt for the next pass. */
> > >  	opterr = 1;
> > >  	optind = 1;
> > > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > >  			do_unload = 1;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_UNLOAD;
> > > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> > >  			break;
> > >  		case OPT_EXEC:
> > >  			do_load = 0;  
> >  @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> > 
> >  The existing code has the following above the context shown in the
> > patch:
> > 
> > 			do_load = 1;
> > 
> > 
> > >  			do_exec = 0;
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_ON_CRASH;
> > > -			else
> > > -				kexec_flags = KEXEC_ON_CRASH;
> > > -			break;
> > > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > > +			kexec_flags = KEXEC_ON_CRASH;  
> > 
> > kexec_flags is now set regardless of the value of
> > do_kexec_file_syscall, which was not the case prior to this patch.
> > That seems to affect the following which appears later in the
> > function. Is that ok?
> > 
> > 
> >         if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
> >             !is_crashkernel_mem_reserved()) {
> >                 die("Memory for crashkernel is not reserved\n"
> >                     "Please reserve memory by passing"
> >                     "\"crashkernel=X@Y\" parameter to kernel\n"
> >                     "Then try to loading kdump kernel\n");
> >         }
> 
> Do you not need memory for kexec -s? This looks broken to start with.

Could you propose a fix? I realise your patchset may not introduce this
problem. But it seems to me that it makes things slightly worse or at
the very least perpetuates the notion that the above is correct.

> > 	...
> > 
> >         if ((result == 0) && do_load_jump_back_helper) {
> 
> And yes, this should not be allowed with -s

Is the simple fix here for your patch to add an extra condition
to the if statement above?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] kexec: do not special-case the -s option

[Michal Suchánek]

On Fri, 16 Mar 2018 12:20:25 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Thu, Mar 15, 2018 at 12:13:18PM +0100, Michal Suchánek wrote:
> > On Thu, 15 Mar 2018 11:38:30 +0100
> > Simon Horman <horms@verge.net.au> wrote:

> > > 
> > > 
> > >         if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
> > >             !is_crashkernel_mem_reserved()) {
> > >                 die("Memory for crashkernel is not reserved\n"
> > >                     "Please reserve memory by passing"
> > >                     "\"crashkernel=X@Y\" parameter to kernel\n"
> > >                     "Then try to loading kdump kernel\n");
> > >         }  
> > 
> > Do you not need memory for kexec -s? This looks broken to start
> > with.  
> 
> Could you propose a fix? I realise your patchset may not introduce
> this problem. But it seems to me that it makes things slightly worse
> or at the very least perpetuates the notion that the above is correct.

Yes, it makes sense to fix the condition.

> 
> > > 	...
> > > 
> > >         if ((result == 0) && do_load_jump_back_helper) {  
> > 
> > And yes, this should not be allowed with -s  
> 
> Is the simple fix here for your patch to add an extra condition
> to the if statement above?

This is not supported with -s but nothing prevents setting the flag. So
a test for kexec_load should be added I guess.

Thanks

Michal


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 4/5] kexec: add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported.

[Michal Suchánek]

On Fri, 16 Mar 2018 14:45:02 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/15/18 at 12:06pm, Michal Suchánek wrote:
> > On Wed, 14 Mar 2018 11:21:59 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >   
> > > It looks to me it is enough only checking -ENOSYS maybe also
> > > -ENOTSUPP and then set do_kexec_file_syscall = 0;
> > > 
> > > EINVAL and ENOEXEC are real errors, I do not understand why still 
> > > fallback.    
> > 
> > If you pass an image type that the kernel does not understand (eg.
> > multiboot or uImage) then the kernel will return a real error
> > because it does not understand the image. However, kexec-tools
> > should still be able to load it, automatically. That's what the
> > -auto stands for.  
> 
> This semes over engineering, the initial purpose is to fallback when
> kexec_file_load is not supported, so I would suggest not to do more
> than that.

The initial purpose is for users to not need to specify any flag to
select a syscall to use. Since this is not acceptable because it might
make kexec work out of the box on systems where it was previously broken
or whatever this new flag is supposed to achieve that: when specified
kexec tries hard to do the best thing possible with the image it
received.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Fri, Mar 16, 2018 at 12:38:13PM +0100, Michal Suchánek wrote:
> On Fri, 16 Mar 2018 12:20:25 +0100
> Simon Horman <horms@verge.net.au> wrote:
> 
> > On Thu, Mar 15, 2018 at 12:13:18PM +0100, Michal Suchánek wrote:
> > > On Thu, 15 Mar 2018 11:38:30 +0100
> > > Simon Horman <horms@verge.net.au> wrote:
> 
> > > > 
> > > > 
> > > >         if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
> > > >             !is_crashkernel_mem_reserved()) {
> > > >                 die("Memory for crashkernel is not reserved\n"
> > > >                     "Please reserve memory by passing"
> > > >                     "\"crashkernel=X@Y\" parameter to kernel\n"
> > > >                     "Then try to loading kdump kernel\n");
> > > >         }  
> > > 
> > > Do you not need memory for kexec -s? This looks broken to start
> > > with.  
> > 
> > Could you propose a fix? I realise your patchset may not introduce
> > this problem. But it seems to me that it makes things slightly worse
> > or at the very least perpetuates the notion that the above is correct.
> 
> Yes, it makes sense to fix the condition.

Thanks.

> > > > 	...
> > > > 
> > > >         if ((result == 0) && do_load_jump_back_helper) {  
> > > 
> > > And yes, this should not be allowed with -s  
> > 
> > Is the simple fix here for your patch to add an extra condition
> > to the if statement above?
> 
> This is not supported with -s but nothing prevents setting the flag. So
> a test for kexec_load should be added I guess.

That is what I was thinking too.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 5/5] kexec: document -s, -c and -a options.

[Michal Suchánek]

On Fri, 16 Mar 2018 14:51:14 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/15/18 at 12:44pm, Michal Suchánek wrote:
> > On Wed, 14 Mar 2018 15:50:31 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >   
> > > On 03/14/18 at 08:25am, Michal Suchánek wrote:  
> > > > On Wed, 14 Mar 2018 11:41:30 +0800
> > > > Dave Young <dyoung@redhat.com> wrote:
> > > >     
> > > > > On 03/06/18 at 02:15pm, Michal Suchanek wrote:    
> > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > > > ---
> > > > > >  kexec/kexec.8 | 15 +++++++++++++++
> > > > > >  1 file changed, 15 insertions(+)
> > > > > > 
> > > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8
> > > > > > index e0131b4ea827..b3543db3f413 100644
> > > > > > --- a/kexec/kexec.8
> > > > > > +++ b/kexec/kexec.8
> > > > > > @@ -144,6 +144,21 @@ Load the new kernel for use on panic.
> > > > > >  Specify that the new kernel is of this
> > > > > >  .I type.
> > > > > >  .TP
> > > > > > +.BI \-s\ (\-\-kexec-file-syscall)
> > > > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used
> > > > > > exclusively.      
> > > > > 
> > > > > Maybe better to be simple like below:
> > > > > "Use kexec_file_load syscall to load the new kernel."
> > > > > 
> > > > >     
> > > > > > +.TP
> > > > > > +.BI \-c\ (\-\-kexec-syscall)
> > > > > > +Specify that the old KEXEC_LOAD syscall should be used
> > > > > > exclusively (the default).      
> > > > > 
> > > > > similarly:
> > > > > "Use kexec_load syscall to load the new kernel."
> > > > >     
> > > > > > +.TP
> > > > > > +.BI \-a\ (\-\-kexec-syscall-auto)
> > > > > > +Try the new simpler KEXEC_FILE_LOAD syscall first and if
> > > > > > it is not supported +fall back to the old KEXEC_LOAD
> > > > > > interface. +
> > > > > > +There is no one single interface that always works.
> > > > > > KEXEC_FILE_LOAD is required +on systems that use locked-down
> > > > > > secure boot to verify the kernel signature. +KEXEC_LOAD is
> > > > > > required for some kernel image formats and on architectures
> > > > > > that +do not support KEXEC_FILE_LOAD.      
> > > > > 
> > > > > It seems not good to say kexec_file_load is simpler and newer.
> > > > > Also it is not a must for Secure Boot and locked down kernel
> > > > > only. So it would be better to just simplify and use the first
> > > > > paragraph:
> > > > > 
> > > > > "Try kexec_file_load syscall first and if it is not supported
> > > > > fall back to the kexec_load syscall"    
> > > > 
> > > > There was a request for explanation so just the first paragraph
> > > > will not do. What is it required for other than secure boot?    
> > > 
> > > People can use kexec -s to load a signed kernel but not necessary
> > > to boot with Secure Boot enabled.  
> > 
> > Is booting signed kernel without -s not supported? If so I would
> > consider it kexec-tools bug. And it should documented then as well I
> > guess.  
> 
> I'm not sure I understand the question.  In kernel we splitted kexec
> and kexec_file they can be enabled as kernel config options
> separately.  If one want to a secured kexec (not UEFI Secure Boot,
> only signed kernel loading) then one can only enable
> CONFIG_KEXEC_FILE but disable CONFIG_KEXEC.  In this case without
> '-s' load will fail.  But if one enabled both CONFIG_KEXEC_FILE and
> CONFIG_KEXEC then kexec load without '-s' still works.

This is currently not supported by kexec-tools. Status does not use
KEXEC_FILE

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v5 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchanek]

When the kernel does not know a syscall number it returns -ENOSYS but
when kexec does not know a syscall number it returns -1. Return -ENOSYS
from kexec as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index cfd837c1b6bb..ab8cff7fe083 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
 
 	if (!is_kexec_file_load_implemented()) {
 		fprintf(stderr, "syscall kexec_file_load not available.\n");
-		return -1;
+		return -ENOSYS;
 	}
 
 	if (argc - fileind <= 0) {
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v5 2/5] kexec: Fix option checks to take KEXEC_FILE_LOAD into account

[Michal Suchanek]

When kexec_file_load support was added some sanity checks were not updated.

Some options are set only in the kexec_load flags so cannot be supported
wiht kexec_file_load. On the other hand, reserved memory is needed for
kdump with both kexec_load and kexec_file_load.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index ab8cff7fe083..b793f31ea501 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1415,7 +1415,9 @@ int main(int argc, char *argv[])
 		do_load_jump_back_helper = 0;
 	}
 
-	if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
+	if (do_load &&
+	    ((kexec_flags & KEXEC_ON_CRASH) ||
+	     (kexec_file_flags & KEXEC_FILE_ON_CRASH)) &&
 	    !is_crashkernel_mem_reserved()) {
 		die("Memory for crashkernel is not reserved\n"
 		    "Please reserve memory by passing"
@@ -1447,6 +1449,12 @@ int main(int argc, char *argv[])
 			}
 		}
 	}
+	if (do_kexec_file_syscall) {
+		if (do_load_jump_back_helper)
+			die("--load-jump-back-helper not supported with kexec_file_load\n");
+		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
+			die("--load-preserve-context not supported with kexec_file_load\n");
+	}
 
 	if (do_reuse_initrd){
 		check_reuse_initrd();
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v5 3/5] kexec: Do not special-case the -s option

[Michal Suchanek]

It is parsed separately to save a few CPU cycles when setting up other
options but it just complicates the code. So fold it back and set up all
flags both for KEXEC_LOAD and KEXEC_FILE_LOAD

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index b793f31ea501..68ae0594d4a7 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
 	};
 	static const char short_options[] = KEXEC_ALL_OPT_STR;
 
-	/*
-	 * First check if --use-kexec-file-syscall is set. That changes lot of
-	 * things
-	 */
-	while ((opt = getopt_long(argc, argv, short_options,
-				  options, 0)) != -1) {
-		switch(opt) {
-		case OPT_KEXEC_FILE_SYSCALL:
-			do_kexec_file_syscall = 1;
-			break;
-		}
-	}
-
 	/* Reset getopt for the next pass. */
 	opterr = 1;
 	optind = 1;
@@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
 			do_shutdown = 0;
 			do_sync = 0;
 			do_unload = 1;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_UNLOAD;
+			kexec_file_flags |= KEXEC_FILE_UNLOAD;
 			break;
 		case OPT_EXEC:
 			do_load = 0;
@@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
 			do_exec = 0;
 			do_shutdown = 0;
 			do_sync = 0;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
-			else
-				kexec_flags = KEXEC_ON_CRASH;
-			break;
+			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
+			kexec_flags = KEXEC_ON_CRASH;
 		case OPT_MEM_MIN:
 			mem_min = strtoul(optarg, &endptr, 0);
 			if (*endptr) {
@@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
 			do_reuse_initrd = 1;
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
-			/* We already parsed it. Nothing to do. */
+			do_kexec_file_syscall = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

So add an -a option that tries KEXEC_FILE_LOAD and when it is not
supported tries KEXEC_LOAD.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: instead of changing the deafult add extra option
v4: actually check -ENOSYS as well
v5: add missing break
---
 kexec/kexec.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 kexec/kexec.h |  6 +++++-
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 68ae0594d4a7..44042345a16e 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 0;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
+			break;
+		case OPT_KEXEC_SYSCALL:
+			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
+			break;
+		case OPT_KEXEC_SYSCALL_AUTO:
+			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
@@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
 		}
 	}
 	if (do_kexec_file_syscall) {
-		if (do_load_jump_back_helper)
+		if (do_load_jump_back_helper && !do_kexec_fallback)
 			die("--load-jump-back-helper not supported with kexec_file_load\n");
 		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
 			die("--load-preserve-context not supported with kexec_file_load\n");
@@ -1447,16 +1457,54 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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:
+					 /*
+					  * ENOTSUPP can be unsupported image
+					  * type or unsupported PE signature
+					  * wrapper type, duh
+					  */
+				case -ENOTSUP:
+					do_kexec_file_syscall = 0;
+					break;
+			}
+		}
+		if (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 26225d2c002a..d445fbe3e486 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -219,6 +219,8 @@ extern int file_types;
 #define OPT_TYPE		't'
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
+#define OPT_KEXEC_SYSCALL	'c'
+#define OPT_KEXEC_SYSCALL_AUTO	'a'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -246,11 +248,13 @@ extern int file_types;
 	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
+	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
+	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:psS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v5 5/5] kexec: Document -s, -c and -a options

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.8 | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/kexec/kexec.8 b/kexec/kexec.8
index e0131b4ea827..0682df06f931 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -144,6 +144,25 @@ Load the new kernel for use on panic.
 Specify that the new kernel is of this
 .I type.
 .TP
+.BI \-s\ (\-\-kexec-file-syscall)
+Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
+.TP
+.BI \-c\ (\-\-kexec-syscall)
+Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).
+.TP
+.BI \-a\ (\-\-kexec-syscall-auto)
+Try the new KEXEC_FILE_LOAD syscall first and if it is not supported fall back
+to the old KEXEC_LOAD interface.
+
+There is no one single interface that always works.
+
+KEXEC_FILE_LOAD is required on systems that use locked-down secure boot to
+verify the kernel signature.  KEXEC_LOAD may be also disabled in the kernel
+configuration.
+
+KEXEC_LOAD is required for some kernel image formats and on architectures that
+do not implement KEXEC_FILE_LOAD.
+.TP
 .B \-u\ (\-\-unload)
 Unload the current
 .B kexec
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Simon Horman]

Hi Michal, thanks for the updated patches.

Dave, are you planning to review this series?

On Tue, Mar 20, 2018 at 04:56:16PM +0100, Michal Suchanek wrote:
> When the kernel does not know a syscall number it returns -ENOSYS but
> when kexec does not know a syscall number it returns -1. Return -ENOSYS
> from kexec as well.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index cfd837c1b6bb..ab8cff7fe083 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
>  
>  	if (!is_kexec_file_load_implemented()) {
>  		fprintf(stderr, "syscall kexec_file_load not available.\n");
> -		return -1;
> +		return -ENOSYS;
>  	}
>  
>  	if (argc - fileind <= 0) {
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Dave Young]

Hi Simon
On 03/26/18 at 09:25am, Simon Horman wrote:
> Hi Michal, thanks for the updated patches.
> 
> Dave, are you planning to review this series?
> 

I have same concern as I commented in last versioni, but seems
we can not convince each other with Michal.

For example for -EINVAL/-ENOEXEC, since it can be some misc
error checking in kernel code, it is not equal to an unsupported
syscall.  I'm not keen to think broken kernel file (include the case for
unsupported kernel format, but not limit to that) is equal as an
unsupported syscall

Also seems the new options are not showing in `kexec -h` although
added in the man page.

So I think I will leave to you and do not object it if you are fine. 

> On Tue, Mar 20, 2018 at 04:56:16PM +0100, Michal Suchanek wrote:
> > When the kernel does not know a syscall number it returns -ENOSYS but
> > when kexec does not know a syscall number it returns -1. Return -ENOSYS
> > from kexec as well.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index cfd837c1b6bb..ab8cff7fe083 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
> >  
> >  	if (!is_kexec_file_load_implemented()) {
> >  		fprintf(stderr, "syscall kexec_file_load not available.\n");
> > -		return -1;
> > +		return -ENOSYS;
> >  	}
> >  
> >  	if (argc - fileind <= 0) {
> > -- 
> > 2.13.6
> > 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Dave Young]

On 03/20/18 at 04:56pm, Michal Suchanek wrote:
> Not all architectures implement KEXEC_FILE_LOAD. However, on some
> archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> syscall with undocumented -s option. However, if they did pass the
> option kexec would fail on architectures that do not support it.
> 
> So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> supported tries KEXEC_LOAD.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v3: instead of changing the deafult add extra option
> v4: actually check -ENOSYS as well
> v5: add missing break
> ---
>  kexec/kexec.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  kexec/kexec.h |  6 +++++-
>  2 files changed, 58 insertions(+), 6 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index 68ae0594d4a7..44042345a16e 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>  	int do_unload = 0;
>  	int do_reuse_initrd = 0;
>  	int do_kexec_file_syscall = 0;
> +	int do_kexec_fallback = 0;
>  	int do_status = 0;
>  	void *entry = 0;
>  	char *type = 0;
> @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
>  			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 0;
> +			break;
> +		case OPT_KEXEC_SYSCALL:
> +			do_kexec_file_syscall = 0;
> +			do_kexec_fallback = 0;
> +			break;
> +		case OPT_KEXEC_SYSCALL_AUTO:
> +			do_kexec_file_syscall = 1;
> +			do_kexec_fallback = 1;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;
> @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  	if (do_kexec_file_syscall) {
> -		if (do_load_jump_back_helper)
> +		if (do_load_jump_back_helper && !do_kexec_fallback)
>  			die("--load-jump-back-helper not supported with kexec_file_load\n");
>  		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
>  			die("--load-preserve-context not supported with kexec_file_load\n");
> @@ -1447,16 +1457,54 @@ int main(int argc, char *argv[])
>  		result = k_status(kexec_flags);
>  	}
>  	if (do_unload) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = kexec_file_unload(kexec_file_flags);
> -		else
> +			if ((result == -ENOSYS) && do_kexec_fallback)
> +				do_kexec_file_syscall = 0;
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = k_unload(kexec_flags);
>  	}
>  	if (do_load && (result == 0)) {
> -		if (do_kexec_file_syscall)
> +		if (do_kexec_file_syscall) {
>  			result = do_kexec_file_load(fileind, argc, argv,
>  						 kexec_file_flags);
> -		else
> +			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:
> +					 /*
> +					  * ENOTSUPP can be unsupported image
> +					  * type or unsupported PE signature
> +					  * wrapper type, duh
> +					  */
> +				case -ENOTSUP:

Hmm, this is still used in latest version.  kernel does not return such
error number,  I might not say clearly previously.  Please check the
kernel code, the only one place I know is because no kdump support in
power kexec_file:
arch/powerpc/kernel/machine_kexec_file_64.c

        /* We don't support crash kernels yet. */
        if (image->type == KEXEC_TYPE_CRASH)
                return -ENOTSUPP;

So I suggest not checking this as well since -ENOTSUPP is not populated
in userspace headers, and -ENOTSUP is not used at all.

Also as I mentioned in another reply -EINVAL and -ENOEXEC is also not
ncessary.

For -ENOTSUP, maybe someone can submit a patch to switch to -ENOTSUPP
so that userspace can check it.
Ccing Thiago and Hari for the -ENOTSUPP errno issue.

> +					do_kexec_file_syscall = 0;
> +					break;
> +			}
> +		}
> +		if (!do_kexec_file_syscall)
>  			result = my_load(type, fileind, argc, argv,
>  						kexec_flags, entry);
>  	}
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 26225d2c002a..d445fbe3e486 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -219,6 +219,8 @@ extern int file_types;
>  #define OPT_TYPE		't'
>  #define OPT_PANIC		'p'
>  #define OPT_KEXEC_FILE_SYSCALL	's'
> +#define OPT_KEXEC_SYSCALL	'c'
> +#define OPT_KEXEC_SYSCALL_AUTO	'a'
>  #define OPT_STATUS		'S'
>  #define OPT_MEM_MIN             256
>  #define OPT_MEM_MAX             257
> @@ -246,11 +248,13 @@ extern int file_types;
>  	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
>  	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
>  	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
> +	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
> +	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
>  	{ "debug",		0, 0, OPT_DEBUG }, \
>  	{ "status",		0, 0, OPT_STATUS }, \
>  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
>  
> -#define KEXEC_OPT_STR "h?vdfxyluet:psS"
> +#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
>  
>  extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
>  extern void die(const char *fmt, ...)
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Dave Young]

On 03/26/18 at 05:08pm, Dave Young wrote:
> On 03/20/18 at 04:56pm, Michal Suchanek wrote:
> > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
> > locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
> > syscall with undocumented -s option. However, if they did pass the
> > option kexec would fail on architectures that do not support it.
> > 
> > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > supported tries KEXEC_LOAD.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v3: instead of changing the deafult add extra option
> > v4: actually check -ENOSYS as well
> > v5: add missing break
> > ---
> >  kexec/kexec.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  kexec/kexec.h |  6 +++++-
> >  2 files changed, 58 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index 68ae0594d4a7..44042345a16e 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >  	int do_unload = 0;
> >  	int do_reuse_initrd = 0;
> >  	int do_kexec_file_syscall = 0;
> > +	int do_kexec_fallback = 0;
> >  	int do_status = 0;
> >  	void *entry = 0;
> >  	char *type = 0;
> > @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
> >  			break;
> >  		case OPT_KEXEC_FILE_SYSCALL:
> >  			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 0;
> > +			break;
> > +		case OPT_KEXEC_SYSCALL:
> > +			do_kexec_file_syscall = 0;
> > +			do_kexec_fallback = 0;
> > +			break;
> > +		case OPT_KEXEC_SYSCALL_AUTO:
> > +			do_kexec_file_syscall = 1;
> > +			do_kexec_fallback = 1;
> >  			break;
> >  		case OPT_STATUS:
> >  			do_status = 1;
> > @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  	if (do_kexec_file_syscall) {
> > -		if (do_load_jump_back_helper)
> > +		if (do_load_jump_back_helper && !do_kexec_fallback)
> >  			die("--load-jump-back-helper not supported with kexec_file_load\n");
> >  		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
> >  			die("--load-preserve-context not supported with kexec_file_load\n");
> > @@ -1447,16 +1457,54 @@ int main(int argc, char *argv[])
> >  		result = k_status(kexec_flags);
> >  	}
> >  	if (do_unload) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = kexec_file_unload(kexec_file_flags);
> > -		else
> > +			if ((result == -ENOSYS) && do_kexec_fallback)
> > +				do_kexec_file_syscall = 0;
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = k_unload(kexec_flags);
> >  	}
> >  	if (do_load && (result == 0)) {
> > -		if (do_kexec_file_syscall)
> > +		if (do_kexec_file_syscall) {
> >  			result = do_kexec_file_load(fileind, argc, argv,
> >  						 kexec_file_flags);
> > -		else
> > +			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:
> > +					 /*
> > +					  * ENOTSUPP can be unsupported image
> > +					  * type or unsupported PE signature
> > +					  * wrapper type, duh
> > +					  */
> > +				case -ENOTSUP:
> 
> Hmm, this is still used in latest version.  kernel does not return such
> error number,  I might not say clearly previously.  Please check the
> kernel code, the only one place I know is because no kdump support in
> power kexec_file:
> arch/powerpc/kernel/machine_kexec_file_64.c
> 
>         /* We don't support crash kernels yet. */
>         if (image->type == KEXEC_TYPE_CRASH)
>                 return -ENOTSUPP;
> 
> So I suggest not checking this as well since -ENOTSUPP is not populated
> in userspace headers, and -ENOTSUP is not used at all.
> 
> Also as I mentioned in another reply -EINVAL and -ENOEXEC is also not
> ncessary.
> 
> For -ENOTSUP, maybe someone can submit a patch to switch to -ENOTSUPP
> so that userspace can check it.
> Ccing Thiago and Hari for the -ENOTSUPP errno issue.

Oops for the hurry reply,  I means -ENOTSUPP might be able to replaced
with -EOPNOTSUPP, a similar change like this:
https://patchwork.kernel.org/patch/8490791/

> 
> > +					do_kexec_file_syscall = 0;
> > +					break;
> > +			}
> > +		}
> > +		if (!do_kexec_file_syscall)
> >  			result = my_load(type, fileind, argc, argv,
> >  						kexec_flags, entry);
> >  	}
> > diff --git a/kexec/kexec.h b/kexec/kexec.h
> > index 26225d2c002a..d445fbe3e486 100644
> > --- a/kexec/kexec.h
> > +++ b/kexec/kexec.h
> > @@ -219,6 +219,8 @@ extern int file_types;
> >  #define OPT_TYPE		't'
> >  #define OPT_PANIC		'p'
> >  #define OPT_KEXEC_FILE_SYSCALL	's'
> > +#define OPT_KEXEC_SYSCALL	'c'
> > +#define OPT_KEXEC_SYSCALL_AUTO	'a'
> >  #define OPT_STATUS		'S'
> >  #define OPT_MEM_MIN             256
> >  #define OPT_MEM_MAX             257
> > @@ -246,11 +248,13 @@ extern int file_types;
> >  	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
> >  	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
> >  	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
> > +	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
> > +	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
> >  	{ "debug",		0, 0, OPT_DEBUG }, \
> >  	{ "status",		0, 0, OPT_STATUS }, \
> >  	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
> >  
> > -#define KEXEC_OPT_STR "h?vdfxyluet:psS"
> > +#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
> >  
> >  extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
> >  extern void die(const char *fmt, ...)
> > -- 
> > 2.13.6
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> 
> Thanks
> Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Michal Suchánek]

On Mon, 26 Mar 2018 17:12:10 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/26/18 at 05:08pm, Dave Young wrote:
> > On 03/20/18 at 04:56pm, Michal Suchanek wrote:  
> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > enabled in locked-down mode. Previously users had to select the
> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > they did pass the option kexec would fail on architectures that
> > > do not support it.
> > > 
> > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > > supported tries KEXEC_LOAD.
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v3: instead of changing the deafult add extra option
> > > v4: actually check -ENOSYS as well
> > > v5: add missing break
> > > ---
> > >  kexec/kexec.c | 58
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> > > kexec/kexec.h |  6 +++++- 2 files changed, 58 insertions(+), 6
> > > deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index 68ae0594d4a7..44042345a16e 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> > >  	int do_unload = 0;
> > >  	int do_reuse_initrd = 0;
> > >  	int do_kexec_file_syscall = 0;
> > > +	int do_kexec_fallback = 0;
> > >  	int do_status = 0;
> > >  	void *entry = 0;
> > >  	char *type = 0;
> > > @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
> > >  			break;
> > >  		case OPT_KEXEC_FILE_SYSCALL:
> > >  			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 0;
> > > +			break;
> > > +		case OPT_KEXEC_SYSCALL:
> > > +			do_kexec_file_syscall = 0;
> > > +			do_kexec_fallback = 0;
> > > +			break;
> > > +		case OPT_KEXEC_SYSCALL_AUTO:
> > > +			do_kexec_file_syscall = 1;
> > > +			do_kexec_fallback = 1;
> > >  			break;
> > >  		case OPT_STATUS:
> > >  			do_status = 1;
> > > @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  	if (do_kexec_file_syscall) {
> > > -		if (do_load_jump_back_helper)
> > > +		if (do_load_jump_back_helper
> > > && !do_kexec_fallback) die("--load-jump-back-helper not supported
> > > with kexec_file_load\n"); if (kexec_flags &
> > > KEXEC_PRESERVE_CONTEXT) die("--load-preserve-context not
> > > supported with kexec_file_load\n"); @@ -1447,16 +1457,54 @@ int
> > > main(int argc, char *argv[]) result = k_status(kexec_flags);
> > >  	}
> > >  	if (do_unload) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result =
> > > kexec_file_unload(kexec_file_flags);
> > > -		else
> > > +			if ((result == -ENOSYS) &&
> > > do_kexec_fallback)
> > > +				do_kexec_file_syscall = 0;
> > > +		}
> > > +		if (!do_kexec_file_syscall)
> > >  			result = k_unload(kexec_flags);
> > >  	}
> > >  	if (do_load && (result == 0)) {
> > > -		if (do_kexec_file_syscall)
> > > +		if (do_kexec_file_syscall) {
> > >  			result = do_kexec_file_load(fileind,
> > > argc, argv, kexec_file_flags);
> > > -		else
> > > +			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:
> > > +					 /*
> > > +					  * ENOTSUPP can be
> > > unsupported image
> > > +					  * type or unsupported
> > > PE signature
> > > +					  * wrapper type, duh
> > > +					  */
> > > +				case -ENOTSUP:  
> > 
> > Hmm, this is still used in latest version.  kernel does not return
> > such error number,  I might not say clearly previously.  Please
> > check the kernel code, the only one place I know is because no
> > kdump support in power kexec_file:
> > arch/powerpc/kernel/machine_kexec_file_64.c
> > 
> >         /* We don't support crash kernels yet. */
> >         if (image->type == KEXEC_TYPE_CRASH)
> >                 return -ENOTSUPP;
> > 
> > So I suggest not checking this as well since -ENOTSUPP is not
> > populated in userspace headers, and -ENOTSUP is not used at all.
> > 
> > Also as I mentioned in another reply -EINVAL and -ENOEXEC is also
> > not ncessary.
> > 
> > For -ENOTSUP, maybe someone can submit a patch to switch to
> > -ENOTSUPP so that userspace can check it.
> > Ccing Thiago and Hari for the -ENOTSUPP errno issue.  
> 
> Oops for the hurry reply,  I means -ENOTSUPP might be able to replaced
> with -EOPNOTSUPP, a similar change like this:
> https://patchwork.kernel.org/patch/8490791/

Thanks for catching this. In Linux ENOTSUPP with extra P is different
from EOPNOTSUPP and ENOTSUP (single P). Since we are talking to the
kernel and it returns the double P ENOTSUPP we need to define it in
kexec as well. And we should check ENOTSUP with single P in case
somebody some day thinks that returning undefined error codes to
userspace is not nice like in the patch above.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchánek]

On Mon, 26 Mar 2018 15:53:24 +0800
Dave Young <dyoung@redhat.com> wrote:

> Hi Simon
> On 03/26/18 at 09:25am, Simon Horman wrote:
> > Hi Michal, thanks for the updated patches.
> > 
> > Dave, are you planning to review this series?
> >   
> 
> I have same concern as I commented in last versioni, but seems
> we can not convince each other with Michal.
> 
> For example for -EINVAL/-ENOEXEC, since it can be some misc
> error checking in kernel code, it is not equal to an unsupported
> syscall.  I'm not keen to think broken kernel file (include the case
> for unsupported kernel format, but not limit to that) is equal as an
> unsupported syscall

I do not say it is equal to unsupported syscall. However, the kernel
cannot really tell if the image is complete garbage or if it is in
format it does not understand. The only way to check that is trying to
load in the old way. I don't see any way around that if we want an
--auto option which is of any use.

> 
> Also seems the new options are not showing in `kexec -h` although
> added in the man page.

Yes, that's another place where the description should be added.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Thiago Jung Bauermann]

Michal Suchánek <msuchanek@suse.de> writes:

> On Mon, 26 Mar 2018 17:12:10 +0800
> Dave Young <dyoung@redhat.com> wrote:
>
>> On 03/26/18 at 05:08pm, Dave Young wrote:
>> > On 03/20/18 at 04:56pm, Michal Suchanek wrote:  
>> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
>> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
>> > > enabled in locked-down mode. Previously users had to select the
>> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
>> > > they did pass the option kexec would fail on architectures that
>> > > do not support it.
>> > > 
>> > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
>> > > supported tries KEXEC_LOAD.
>> > > 
>> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>> > > ---
>> > > v3: instead of changing the deafult add extra option
>> > > v4: actually check -ENOSYS as well
>> > > v5: add missing break
>> > > ---
>> > >  kexec/kexec.c | 58
>> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> > > kexec/kexec.h |  6 +++++- 2 files changed, 58 insertions(+), 6
>> > > deletions(-)
>> > > 
>> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
>> > > index 68ae0594d4a7..44042345a16e 100644
>> > > --- a/kexec/kexec.c
>> > > +++ b/kexec/kexec.c
>> > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
>> > >  	int do_unload = 0;
>> > >  	int do_reuse_initrd = 0;
>> > >  	int do_kexec_file_syscall = 0;
>> > > +	int do_kexec_fallback = 0;
>> > >  	int do_status = 0;
>> > >  	void *entry = 0;
>> > >  	char *type = 0;
>> > > @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
>> > >  			break;
>> > >  		case OPT_KEXEC_FILE_SYSCALL:
>> > >  			do_kexec_file_syscall = 1;
>> > > +			do_kexec_fallback = 0;
>> > > +			break;
>> > > +		case OPT_KEXEC_SYSCALL:
>> > > +			do_kexec_file_syscall = 0;
>> > > +			do_kexec_fallback = 0;
>> > > +			break;
>> > > +		case OPT_KEXEC_SYSCALL_AUTO:
>> > > +			do_kexec_file_syscall = 1;
>> > > +			do_kexec_fallback = 1;
>> > >  			break;
>> > >  		case OPT_STATUS:
>> > >  			do_status = 1;
>> > > @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
>> > >  		}
>> > >  	}
>> > >  	if (do_kexec_file_syscall) {
>> > > -		if (do_load_jump_back_helper)
>> > > +		if (do_load_jump_back_helper
>> > > && !do_kexec_fallback) die("--load-jump-back-helper not supported
>> > > with kexec_file_load\n"); if (kexec_flags &
>> > > KEXEC_PRESERVE_CONTEXT) die("--load-preserve-context not
>> > > supported with kexec_file_load\n"); @@ -1447,16 +1457,54 @@ int
>> > > main(int argc, char *argv[]) result = k_status(kexec_flags);
>> > >  	}
>> > >  	if (do_unload) {
>> > > -		if (do_kexec_file_syscall)
>> > > +		if (do_kexec_file_syscall) {
>> > >  			result =
>> > > kexec_file_unload(kexec_file_flags);
>> > > -		else
>> > > +			if ((result == -ENOSYS) &&
>> > > do_kexec_fallback)
>> > > +				do_kexec_file_syscall = 0;
>> > > +		}
>> > > +		if (!do_kexec_file_syscall)
>> > >  			result = k_unload(kexec_flags);
>> > >  	}
>> > >  	if (do_load && (result == 0)) {
>> > > -		if (do_kexec_file_syscall)
>> > > +		if (do_kexec_file_syscall) {
>> > >  			result = do_kexec_file_load(fileind,
>> > > argc, argv, kexec_file_flags);
>> > > -		else
>> > > +			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:
>> > > +					 /*
>> > > +					  * ENOTSUPP can be
>> > > unsupported image
>> > > +					  * type or unsupported
>> > > PE signature
>> > > +					  * wrapper type, duh
>> > > +					  */
>> > > +				case -ENOTSUP:  
>> > 
>> > Hmm, this is still used in latest version.  kernel does not return
>> > such error number,  I might not say clearly previously.  Please
>> > check the kernel code, the only one place I know is because no
>> > kdump support in power kexec_file:
>> > arch/powerpc/kernel/machine_kexec_file_64.c
>> > 
>> >         /* We don't support crash kernels yet. */
>> >         if (image->type == KEXEC_TYPE_CRASH)
>> >                 return -ENOTSUPP;
>> > 
>> > So I suggest not checking this as well since -ENOTSUPP is not
>> > populated in userspace headers, and -ENOTSUP is not used at all.
>> > 
>> > Also as I mentioned in another reply -EINVAL and -ENOEXEC is also
>> > not ncessary.
>> > 
>> > For -ENOTSUP, maybe someone can submit a patch to switch to
>> > -ENOTSUPP so that userspace can check it.
>> > Ccing Thiago and Hari for the -ENOTSUPP errno issue.  
>> 
>> Oops for the hurry reply,  I means -ENOTSUPP might be able to replaced
>> with -EOPNOTSUPP, a similar change like this:
>> https://patchwork.kernel.org/patch/8490791/
>
> Thanks for catching this. In Linux ENOTSUPP with extra P is different
> from EOPNOTSUPP and ENOTSUP (single P). Since we are talking to the
> kernel and it returns the double P ENOTSUPP we need to define it in
> kexec as well. And we should check ENOTSUP with single P in case
> somebody some day thinks that returning undefined error codes to
> userspace is not nice like in the patch above.

I wasn't aware that ENOTSUPP was an in-kernel only errno. Should I
submit a patch for the kernel so that powerpc returns -EOPNOTSUPP in
case of trying to load kdump kernel with kexec_file_load()?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Michal Suchánek]

On Mon, 26 Mar 2018 15:52:39 -0300
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:

> Michal Suchánek <msuchanek@suse.de> writes:
> 
> > On Mon, 26 Mar 2018 17:12:10 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >  
> >> On 03/26/18 at 05:08pm, Dave Young wrote:  
> >> > On 03/20/18 at 04:56pm, Michal Suchanek wrote:    

> >> > Hmm, this is still used in latest version.  kernel does not
> >> > return such error number,  I might not say clearly previously.
> >> > Please check the kernel code, the only one place I know is
> >> > because no kdump support in power kexec_file:
> >> > arch/powerpc/kernel/machine_kexec_file_64.c
> >> > 
> >> >         /* We don't support crash kernels yet. */
> >> >         if (image->type == KEXEC_TYPE_CRASH)
> >> >                 return -ENOTSUPP;
> >> > 
> >> > So I suggest not checking this as well since -ENOTSUPP is not
> >> > populated in userspace headers, and -ENOTSUP is not used at all.
> >> > 
...
> >> > For -ENOTSUP, maybe someone can submit a patch to switch to
> >> > -ENOTSUPP so that userspace can check it.
> >> > Ccing Thiago and Hari for the -ENOTSUPP errno issue.    
> >> 
> >> Oops for the hurry reply,  I means -ENOTSUPP might be able to
> >> replaced with -EOPNOTSUPP, a similar change like this:
> >> https://patchwork.kernel.org/patch/8490791/  
> >
> > Thanks for catching this. In Linux ENOTSUPP with extra P is
> > different from EOPNOTSUPP and ENOTSUP (single P). Since we are
> > talking to the kernel and it returns the double P ENOTSUPP we need
> > to define it in kexec as well. And we should check ENOTSUP with
> > single P in case somebody some day thinks that returning undefined
> > error codes to userspace is not nice like in the patch above.  
> 
> I wasn't aware that ENOTSUPP was an in-kernel only errno. Should I
> submit a patch for the kernel so that powerpc returns -EOPNOTSUPP in
> case of trying to load kdump kernel with kexec_file_load()?
> 

It does not help us much because we need to support kernels without the
patch as well. In the long run it might be nice to eliminate the return
value in user-facing functions. But it's difficult to tell which
functions are user-facing. The return value can be forwarded quite a
few times before it reaches the user. 

This is far from the only place returning this error code to userspace.
I wonder what was the reason for introducing this error code in the
first place.

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Dave Young]

On 03/26/18 at 08:17pm, Michal Suchánek wrote:
> On Mon, 26 Mar 2018 15:53:24 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > Hi Simon
> > On 03/26/18 at 09:25am, Simon Horman wrote:
> > > Hi Michal, thanks for the updated patches.
> > > 
> > > Dave, are you planning to review this series?
> > >   
> > 
> > I have same concern as I commented in last versioni, but seems
> > we can not convince each other with Michal.
> > 
> > For example for -EINVAL/-ENOEXEC, since it can be some misc
> > error checking in kernel code, it is not equal to an unsupported
> > syscall.  I'm not keen to think broken kernel file (include the case
> > for unsupported kernel format, but not limit to that) is equal as an
> > unsupported syscall
> 
> I do not say it is equal to unsupported syscall. However, the kernel
> cannot really tell if the image is complete garbage or if it is in
> format it does not understand. The only way to check that is trying to
> load in the old way. I don't see any way around that if we want an
> --auto option which is of any use.

Maybe it is some personal taste, I tend to only checking syscall
supported so that it can be simpler.  And maybe from the beginning I
understand the "supported" as only for the syscall itself..

Anyway as having said before let's see how Simon think about this. 

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Dave Young]

On 03/26/18 at 03:52pm, Thiago Jung Bauermann wrote:
> 
> Michal Suchánek <msuchanek@suse.de> writes:
> 
> > On Mon, 26 Mar 2018 17:12:10 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >
> >> On 03/26/18 at 05:08pm, Dave Young wrote:
> >> > On 03/20/18 at 04:56pm, Michal Suchanek wrote:  
> >> > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> >> > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> >> > > enabled in locked-down mode. Previously users had to select the
> >> > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> >> > > they did pass the option kexec would fail on architectures that
> >> > > do not support it.
> >> > > 
> >> > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> >> > > supported tries KEXEC_LOAD.
> >> > > 
> >> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> >> > > ---
> >> > > v3: instead of changing the deafult add extra option
> >> > > v4: actually check -ENOSYS as well
> >> > > v5: add missing break
> >> > > ---
> >> > >  kexec/kexec.c | 58
> >> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >> > > kexec/kexec.h |  6 +++++- 2 files changed, 58 insertions(+), 6
> >> > > deletions(-)
> >> > > 
> >> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> >> > > index 68ae0594d4a7..44042345a16e 100644
> >> > > --- a/kexec/kexec.c
> >> > > +++ b/kexec/kexec.c
> >> > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> >> > >  	int do_unload = 0;
> >> > >  	int do_reuse_initrd = 0;
> >> > >  	int do_kexec_file_syscall = 0;
> >> > > +	int do_kexec_fallback = 0;
> >> > >  	int do_status = 0;
> >> > >  	void *entry = 0;
> >> > >  	char *type = 0;
> >> > > @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
> >> > >  			break;
> >> > >  		case OPT_KEXEC_FILE_SYSCALL:
> >> > >  			do_kexec_file_syscall = 1;
> >> > > +			do_kexec_fallback = 0;
> >> > > +			break;
> >> > > +		case OPT_KEXEC_SYSCALL:
> >> > > +			do_kexec_file_syscall = 0;
> >> > > +			do_kexec_fallback = 0;
> >> > > +			break;
> >> > > +		case OPT_KEXEC_SYSCALL_AUTO:
> >> > > +			do_kexec_file_syscall = 1;
> >> > > +			do_kexec_fallback = 1;
> >> > >  			break;
> >> > >  		case OPT_STATUS:
> >> > >  			do_status = 1;
> >> > > @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
> >> > >  		}
> >> > >  	}
> >> > >  	if (do_kexec_file_syscall) {
> >> > > -		if (do_load_jump_back_helper)
> >> > > +		if (do_load_jump_back_helper
> >> > > && !do_kexec_fallback) die("--load-jump-back-helper not supported
> >> > > with kexec_file_load\n"); if (kexec_flags &
> >> > > KEXEC_PRESERVE_CONTEXT) die("--load-preserve-context not
> >> > > supported with kexec_file_load\n"); @@ -1447,16 +1457,54 @@ int
> >> > > main(int argc, char *argv[]) result = k_status(kexec_flags);
> >> > >  	}
> >> > >  	if (do_unload) {
> >> > > -		if (do_kexec_file_syscall)
> >> > > +		if (do_kexec_file_syscall) {
> >> > >  			result =
> >> > > kexec_file_unload(kexec_file_flags);
> >> > > -		else
> >> > > +			if ((result == -ENOSYS) &&
> >> > > do_kexec_fallback)
> >> > > +				do_kexec_file_syscall = 0;
> >> > > +		}
> >> > > +		if (!do_kexec_file_syscall)
> >> > >  			result = k_unload(kexec_flags);
> >> > >  	}
> >> > >  	if (do_load && (result == 0)) {
> >> > > -		if (do_kexec_file_syscall)
> >> > > +		if (do_kexec_file_syscall) {
> >> > >  			result = do_kexec_file_load(fileind,
> >> > > argc, argv, kexec_file_flags);
> >> > > -		else
> >> > > +			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:
> >> > > +					 /*
> >> > > +					  * ENOTSUPP can be
> >> > > unsupported image
> >> > > +					  * type or unsupported
> >> > > PE signature
> >> > > +					  * wrapper type, duh
> >> > > +					  */
> >> > > +				case -ENOTSUP:  
> >> > 
> >> > Hmm, this is still used in latest version.  kernel does not return
> >> > such error number,  I might not say clearly previously.  Please
> >> > check the kernel code, the only one place I know is because no
> >> > kdump support in power kexec_file:
> >> > arch/powerpc/kernel/machine_kexec_file_64.c
> >> > 
> >> >         /* We don't support crash kernels yet. */
> >> >         if (image->type == KEXEC_TYPE_CRASH)
> >> >                 return -ENOTSUPP;
> >> > 
> >> > So I suggest not checking this as well since -ENOTSUPP is not
> >> > populated in userspace headers, and -ENOTSUP is not used at all.
> >> > 
> >> > Also as I mentioned in another reply -EINVAL and -ENOEXEC is also
> >> > not ncessary.
> >> > 
> >> > For -ENOTSUP, maybe someone can submit a patch to switch to
> >> > -ENOTSUPP so that userspace can check it.
> >> > Ccing Thiago and Hari for the -ENOTSUPP errno issue.  
> >> 
> >> Oops for the hurry reply,  I means -ENOTSUPP might be able to replaced
> >> with -EOPNOTSUPP, a similar change like this:
> >> https://patchwork.kernel.org/patch/8490791/
> >
> > Thanks for catching this. In Linux ENOTSUPP with extra P is different
> > from EOPNOTSUPP and ENOTSUP (single P). Since we are talking to the
> > kernel and it returns the double P ENOTSUPP we need to define it in
> > kexec as well. And we should check ENOTSUP with single P in case
> > somebody some day thinks that returning undefined error codes to
> > userspace is not nice like in the patch above.
> 
> I wasn't aware that ENOTSUPP was an in-kernel only errno. Should I
> submit a patch for the kernel so that powerpc returns -EOPNOTSUPP in
> case of trying to load kdump kernel with kexec_file_load()?

This should be the easiest way and worth a try since userspace check it
now. 

> 
> -- 
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Dave Young]

On 03/26/18 at 07:38pm, Michal Suchánek wrote:
> On Mon, 26 Mar 2018 17:12:10 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/26/18 at 05:08pm, Dave Young wrote:
> > > On 03/20/18 at 04:56pm, Michal Suchanek wrote:  
> > > > Not all architectures implement KEXEC_FILE_LOAD. However, on some
> > > > archiectures KEXEC_FILE_LOAD is required when secure boot is
> > > > enabled in locked-down mode. Previously users had to select the
> > > > KEXEC_FILE_LOAD syscall with undocumented -s option. However, if
> > > > they did pass the option kexec would fail on architectures that
> > > > do not support it.
> > > > 
> > > > So add an -a option that tries KEXEC_FILE_LOAD and when it is not
> > > > supported tries KEXEC_LOAD.
> > > > 
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > > v3: instead of changing the deafult add extra option
> > > > v4: actually check -ENOSYS as well
> > > > v5: add missing break
> > > > ---
> > > >  kexec/kexec.c | 58
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> > > > kexec/kexec.h |  6 +++++- 2 files changed, 58 insertions(+), 6
> > > > deletions(-)
> > > > 
> > > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > > index 68ae0594d4a7..44042345a16e 100644
> > > > --- a/kexec/kexec.c
> > > > +++ b/kexec/kexec.c
> > > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[])
> > > >  	int do_unload = 0;
> > > >  	int do_reuse_initrd = 0;
> > > >  	int do_kexec_file_syscall = 0;
> > > > +	int do_kexec_fallback = 0;
> > > >  	int do_status = 0;
> > > >  	void *entry = 0;
> > > >  	char *type = 0;
> > > > @@ -1367,6 +1368,15 @@ int main(int argc, char *argv[])
> > > >  			break;
> > > >  		case OPT_KEXEC_FILE_SYSCALL:
> > > >  			do_kexec_file_syscall = 1;
> > > > +			do_kexec_fallback = 0;
> > > > +			break;
> > > > +		case OPT_KEXEC_SYSCALL:
> > > > +			do_kexec_file_syscall = 0;
> > > > +			do_kexec_fallback = 0;
> > > > +			break;
> > > > +		case OPT_KEXEC_SYSCALL_AUTO:
> > > > +			do_kexec_file_syscall = 1;
> > > > +			do_kexec_fallback = 1;
> > > >  			break;
> > > >  		case OPT_STATUS:
> > > >  			do_status = 1;
> > > > @@ -1433,7 +1443,7 @@ int main(int argc, char *argv[])
> > > >  		}
> > > >  	}
> > > >  	if (do_kexec_file_syscall) {
> > > > -		if (do_load_jump_back_helper)
> > > > +		if (do_load_jump_back_helper
> > > > && !do_kexec_fallback) die("--load-jump-back-helper not supported
> > > > with kexec_file_load\n"); if (kexec_flags &
> > > > KEXEC_PRESERVE_CONTEXT) die("--load-preserve-context not
> > > > supported with kexec_file_load\n"); @@ -1447,16 +1457,54 @@ int
> > > > main(int argc, char *argv[]) result = k_status(kexec_flags);
> > > >  	}
> > > >  	if (do_unload) {
> > > > -		if (do_kexec_file_syscall)
> > > > +		if (do_kexec_file_syscall) {
> > > >  			result =
> > > > kexec_file_unload(kexec_file_flags);
> > > > -		else
> > > > +			if ((result == -ENOSYS) &&
> > > > do_kexec_fallback)
> > > > +				do_kexec_file_syscall = 0;
> > > > +		}
> > > > +		if (!do_kexec_file_syscall)
> > > >  			result = k_unload(kexec_flags);
> > > >  	}
> > > >  	if (do_load && (result == 0)) {
> > > > -		if (do_kexec_file_syscall)
> > > > +		if (do_kexec_file_syscall) {
> > > >  			result = do_kexec_file_load(fileind,
> > > > argc, argv, kexec_file_flags);
> > > > -		else
> > > > +			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:
> > > > +					 /*
> > > > +					  * ENOTSUPP can be
> > > > unsupported image
> > > > +					  * type or unsupported
> > > > PE signature
> > > > +					  * wrapper type, duh
> > > > +					  */
> > > > +				case -ENOTSUP:  
> > > 
> > > Hmm, this is still used in latest version.  kernel does not return
> > > such error number,  I might not say clearly previously.  Please
> > > check the kernel code, the only one place I know is because no
> > > kdump support in power kexec_file:
> > > arch/powerpc/kernel/machine_kexec_file_64.c
> > > 
> > >         /* We don't support crash kernels yet. */
> > >         if (image->type == KEXEC_TYPE_CRASH)
> > >                 return -ENOTSUPP;
> > > 
> > > So I suggest not checking this as well since -ENOTSUPP is not
> > > populated in userspace headers, and -ENOTSUP is not used at all.
> > > 
> > > Also as I mentioned in another reply -EINVAL and -ENOEXEC is also
> > > not ncessary.
> > > 
> > > For -ENOTSUP, maybe someone can submit a patch to switch to
> > > -ENOTSUPP so that userspace can check it.
> > > Ccing Thiago and Hari for the -ENOTSUPP errno issue.  
> > 
> > Oops for the hurry reply,  I means -ENOTSUPP might be able to replaced
> > with -EOPNOTSUPP, a similar change like this:
> > https://patchwork.kernel.org/patch/8490791/
> 
> Thanks for catching this. In Linux ENOTSUPP with extra P is different
> from EOPNOTSUPP and ENOTSUP (single P). Since we are talking to the
> kernel and it returns the double P ENOTSUPP we need to define it in
> kexec as well. And we should check ENOTSUP with single P in case
> somebody some day thinks that returning undefined error codes to
> userspace is not nice like in the patch above.

I'm not sure if we can define it in kexec-tools since they are used
in kernel only.. 

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Michal Suchánek]

On Tue, 27 Mar 2018 18:06:19 +0800
Dave Young <dyoung@redhat.com> wrote:

> On 03/26/18 at 07:38pm, Michal Suchánek wrote:
> > On Mon, 26 Mar 2018 17:12:10 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> >   
> > > On 03/26/18 at 05:08pm, Dave Young wrote:  
> > > > On 03/20/18 at 04:56pm, Michal Suchanek wrote:    

> > > > Hmm, this is still used in latest version.  kernel does not
> > > > return such error number,  I might not say clearly previously.
> > > > Please check the kernel code, the only one place I know is
> > > > because no kdump support in power kexec_file:
> > > > arch/powerpc/kernel/machine_kexec_file_64.c
> > > > 
> > > >         /* We don't support crash kernels yet. */
> > > >         if (image->type == KEXEC_TYPE_CRASH)
> > > >                 return -ENOTSUPP;
> > > > 
> > > > So I suggest not checking this as well since -ENOTSUPP is not
> > > > populated in userspace headers, and -ENOTSUP is not used at all.
> > > > 
> > > > Also as I mentioned in another reply -EINVAL and -ENOEXEC is
> > > > also not ncessary.
> > > > 
> > > > For -ENOTSUP, maybe someone can submit a patch to switch to
> > > > -ENOTSUPP so that userspace can check it.
> > > > Ccing Thiago and Hari for the -ENOTSUPP errno issue.    
> > > 
> > > Oops for the hurry reply,  I means -ENOTSUPP might be able to
> > > replaced with -EOPNOTSUPP, a similar change like this:
> > > https://patchwork.kernel.org/patch/8490791/  
> > 
> > Thanks for catching this. In Linux ENOTSUPP with extra P is
> > different from EOPNOTSUPP and ENOTSUP (single P). Since we are
> > talking to the kernel and it returns the double P ENOTSUPP we need
> > to define it in kexec as well. And we should check ENOTSUP with
> > single P in case somebody some day thinks that returning undefined
> > error codes to userspace is not nice like in the patch above.  
> 
> I'm not sure if we can define it in kexec-tools since they are used
> in kernel only.. 

We define the KEXEC_FILE_LOAD syscall numbers so why not ENOTSUPP?

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Petr Tesarik]

On Tue, 27 Mar 2018 13:01:29 +0200
Michal Suchánek <msuchanek@suse.de> wrote:

> On Tue, 27 Mar 2018 18:06:19 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/26/18 at 07:38pm, Michal Suchánek wrote:  
> > > On Mon, 26 Mar 2018 17:12:10 +0800
> > > Dave Young <dyoung@redhat.com> wrote:
> > >     
> > > > On 03/26/18 at 05:08pm, Dave Young wrote:    
> > > > > On 03/20/18 at 04:56pm, Michal Suchanek wrote:      
> 
> > > > > Hmm, this is still used in latest version.  kernel does not
> > > > > return such error number,  I might not say clearly previously.
> > > > > Please check the kernel code, the only one place I know is
> > > > > because no kdump support in power kexec_file:
> > > > > arch/powerpc/kernel/machine_kexec_file_64.c
> > > > > 
> > > > >         /* We don't support crash kernels yet. */
> > > > >         if (image->type == KEXEC_TYPE_CRASH)
> > > > >                 return -ENOTSUPP;
> > > > > 
> > > > > So I suggest not checking this as well since -ENOTSUPP is not
> > > > > populated in userspace headers, and -ENOTSUP is not used at all.
> > > > > 
> > > > > Also as I mentioned in another reply -EINVAL and -ENOEXEC is
> > > > > also not ncessary.
> > > > > 
> > > > > For -ENOTSUP, maybe someone can submit a patch to switch to
> > > > > -ENOTSUPP so that userspace can check it.
> > > > > Ccing Thiago and Hari for the -ENOTSUPP errno issue.      
> > > > 
> > > > Oops for the hurry reply,  I means -ENOTSUPP might be able to
> > > > replaced with -EOPNOTSUPP, a similar change like this:
> > > > https://patchwork.kernel.org/patch/8490791/    
> > > 
> > > Thanks for catching this. In Linux ENOTSUPP with extra P is
> > > different from EOPNOTSUPP and ENOTSUP (single P). Since we are
> > > talking to the kernel and it returns the double P ENOTSUPP we need
> > > to define it in kexec as well. And we should check ENOTSUP with
> > > single P in case somebody some day thinks that returning undefined
> > > error codes to userspace is not nice like in the patch above.    
> > 
> > I'm not sure if we can define it in kexec-tools since they are used
> > in kernel only..   
> 
> We define the KEXEC_FILE_LOAD syscall numbers so why not ENOTSUPP?

Essentially, because syscall numbers will be added to Linux uapi
sooner or later, but ENOTSUPP is meant to stay kernel-internal. It is
declared in include/linux/errno.h, which says:

 * These should never be seen by user programs. 

In other words, it is already a kernel bug that these error codes can
are currently returned to user space.

Petr T

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Dave Young]

On 03/27/18 at 01:01pm, Michal Suchánek wrote:
> On Tue, 27 Mar 2018 18:06:19 +0800
> Dave Young <dyoung@redhat.com> wrote:
> 
> > On 03/26/18 at 07:38pm, Michal Suchánek wrote:
> > > On Mon, 26 Mar 2018 17:12:10 +0800
> > > Dave Young <dyoung@redhat.com> wrote:
> > >   
> > > > On 03/26/18 at 05:08pm, Dave Young wrote:  
> > > > > On 03/20/18 at 04:56pm, Michal Suchanek wrote:    
> 
> > > > > Hmm, this is still used in latest version.  kernel does not
> > > > > return such error number,  I might not say clearly previously.
> > > > > Please check the kernel code, the only one place I know is
> > > > > because no kdump support in power kexec_file:
> > > > > arch/powerpc/kernel/machine_kexec_file_64.c
> > > > > 
> > > > >         /* We don't support crash kernels yet. */
> > > > >         if (image->type == KEXEC_TYPE_CRASH)
> > > > >                 return -ENOTSUPP;
> > > > > 
> > > > > So I suggest not checking this as well since -ENOTSUPP is not
> > > > > populated in userspace headers, and -ENOTSUP is not used at all.
> > > > > 
> > > > > Also as I mentioned in another reply -EINVAL and -ENOEXEC is
> > > > > also not ncessary.
> > > > > 
> > > > > For -ENOTSUP, maybe someone can submit a patch to switch to
> > > > > -ENOTSUPP so that userspace can check it.
> > > > > Ccing Thiago and Hari for the -ENOTSUPP errno issue.    
> > > > 
> > > > Oops for the hurry reply,  I means -ENOTSUPP might be able to
> > > > replaced with -EOPNOTSUPP, a similar change like this:
> > > > https://patchwork.kernel.org/patch/8490791/  
> > > 
> > > Thanks for catching this. In Linux ENOTSUPP with extra P is
> > > different from EOPNOTSUPP and ENOTSUP (single P). Since we are
> > > talking to the kernel and it returns the double P ENOTSUPP we need
> > > to define it in kexec as well. And we should check ENOTSUP with
> > > single P in case somebody some day thinks that returning undefined
> > > error codes to userspace is not nice like in the patch above.  
> > 
> > I'm not sure if we can define it in kexec-tools since they are used
> > in kernel only.. 
> 
> We define the KEXEC_FILE_LOAD syscall numbers so why not ENOTSUPP?

They are different syscall number is persistent but internal errnos are
not supposed to be so.

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v5 4/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Simon Horman]

On Wed, Mar 28, 2018 at 08:53:53AM +0800, Dave Young wrote:
> On 03/27/18 at 01:01pm, Michal Suchánek wrote:
> > On Tue, 27 Mar 2018 18:06:19 +0800
> > Dave Young <dyoung@redhat.com> wrote:
> > 
> > > On 03/26/18 at 07:38pm, Michal Suchánek wrote:
> > > > On Mon, 26 Mar 2018 17:12:10 +0800
> > > > Dave Young <dyoung@redhat.com> wrote:
> > > >   
> > > > > On 03/26/18 at 05:08pm, Dave Young wrote:  
> > > > > > On 03/20/18 at 04:56pm, Michal Suchanek wrote:    
> > 
> > > > > > Hmm, this is still used in latest version.  kernel does not
> > > > > > return such error number,  I might not say clearly previously.
> > > > > > Please check the kernel code, the only one place I know is
> > > > > > because no kdump support in power kexec_file:
> > > > > > arch/powerpc/kernel/machine_kexec_file_64.c
> > > > > > 
> > > > > >         /* We don't support crash kernels yet. */
> > > > > >         if (image->type == KEXEC_TYPE_CRASH)
> > > > > >                 return -ENOTSUPP;
> > > > > > 
> > > > > > So I suggest not checking this as well since -ENOTSUPP is not
> > > > > > populated in userspace headers, and -ENOTSUP is not used at all.
> > > > > > 
> > > > > > Also as I mentioned in another reply -EINVAL and -ENOEXEC is
> > > > > > also not ncessary.
> > > > > > 
> > > > > > For -ENOTSUP, maybe someone can submit a patch to switch to
> > > > > > -ENOTSUPP so that userspace can check it.
> > > > > > Ccing Thiago and Hari for the -ENOTSUPP errno issue.    
> > > > > 
> > > > > Oops for the hurry reply,  I means -ENOTSUPP might be able to
> > > > > replaced with -EOPNOTSUPP, a similar change like this:
> > > > > https://patchwork.kernel.org/patch/8490791/  
> > > > 
> > > > Thanks for catching this. In Linux ENOTSUPP with extra P is
> > > > different from EOPNOTSUPP and ENOTSUP (single P). Since we are
> > > > talking to the kernel and it returns the double P ENOTSUPP we need
> > > > to define it in kexec as well. And we should check ENOTSUP with
> > > > single P in case somebody some day thinks that returning undefined
> > > > error codes to userspace is not nice like in the patch above.  
> > > 
> > > I'm not sure if we can define it in kexec-tools since they are used
> > > in kernel only.. 
> > 
> > We define the KEXEC_FILE_LOAD syscall numbers so why not ENOTSUPP?
> 
> They are different syscall number is persistent but internal errnos are
> not supposed to be so.

It sounds to me that the kernel needs to be updated and that upstream
kexec-tools can't support ENOTSUPP.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v6 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Michal Suchanek]

When the kernel does not know a syscall number it returns -ENOSYS but
when kexec does not know a syscall number it returns -1. Return -ENOSYS
from kexec as well.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index cfd837c1b6bb..ab8cff7fe083 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
 
 	if (!is_kexec_file_load_implemented()) {
 		fprintf(stderr, "syscall kexec_file_load not available.\n");
-		return -1;
+		return -ENOSYS;
 	}
 
 	if (argc - fileind <= 0) {
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v6 2/5] kexec: Fix option checks to take KEXEC_FILE_LOAD into account

[Michal Suchanek]

When kexec_file_load support was added some sanity checks were not updated.

Some options are set only in the kexec_load flags so cannot be supported
wiht kexec_file_load. On the other hand, reserved memory is needed for
kdump with both kexec_load and kexec_file_load.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
Added in v5
---
 kexec/kexec.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index ab8cff7fe083..b793f31ea501 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1415,7 +1415,9 @@ int main(int argc, char *argv[])
 		do_load_jump_back_helper = 0;
 	}
 
-	if (do_load && (kexec_flags & KEXEC_ON_CRASH) &&
+	if (do_load &&
+	    ((kexec_flags & KEXEC_ON_CRASH) ||
+	     (kexec_file_flags & KEXEC_FILE_ON_CRASH)) &&
 	    !is_crashkernel_mem_reserved()) {
 		die("Memory for crashkernel is not reserved\n"
 		    "Please reserve memory by passing"
@@ -1447,6 +1449,12 @@ int main(int argc, char *argv[])
 			}
 		}
 	}
+	if (do_kexec_file_syscall) {
+		if (do_load_jump_back_helper)
+			die("--load-jump-back-helper not supported with kexec_file_load\n");
+		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
+			die("--load-preserve-context not supported with kexec_file_load\n");
+	}
 
 	if (do_reuse_initrd){
 		check_reuse_initrd();
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v6 3/5] kexec: Do not special-case the -s option

[Michal Suchanek]

It is parsed separately to save a few CPU cycles when setting up other
options but it just complicates the code. So fold it back and set up all
flags for both KEXEC_LOAD and KEXEC_FILE_LOAD

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 kexec/kexec.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index b793f31ea501..68ae0594d4a7 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
 	};
 	static const char short_options[] = KEXEC_ALL_OPT_STR;
 
-	/*
-	 * First check if --use-kexec-file-syscall is set. That changes lot of
-	 * things
-	 */
-	while ((opt = getopt_long(argc, argv, short_options,
-				  options, 0)) != -1) {
-		switch(opt) {
-		case OPT_KEXEC_FILE_SYSCALL:
-			do_kexec_file_syscall = 1;
-			break;
-		}
-	}
-
 	/* Reset getopt for the next pass. */
 	opterr = 1;
 	optind = 1;
@@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
 			do_shutdown = 0;
 			do_sync = 0;
 			do_unload = 1;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_UNLOAD;
+			kexec_file_flags |= KEXEC_FILE_UNLOAD;
 			break;
 		case OPT_EXEC:
 			do_load = 0;
@@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
 			do_exec = 0;
 			do_shutdown = 0;
 			do_sync = 0;
-			if (do_kexec_file_syscall)
-				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
-			else
-				kexec_flags = KEXEC_ON_CRASH;
-			break;
+			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
+			kexec_flags = KEXEC_ON_CRASH;
 		case OPT_MEM_MIN:
 			mem_min = strtoul(optarg, &endptr, 0);
 			if (*endptr) {
@@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
 			do_reuse_initrd = 1;
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
-			/* We already parsed it. Nothing to do. */
+			do_kexec_file_syscall = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v6 4/5] kexec: Add option to revert -s

[Michal Suchanek]

The undocumented -s option selects KEXEC_FILE_LOAD syscall but there is
no option to select KEXEC_LOAD syscall. Add it so -s can be reverted.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v6: add description to help text
---
 kexec/kexec.c | 5 +++++
 kexec/kexec.h | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 68ae0594d4a7..87689311af2f 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1005,6 +1005,8 @@ void usage(void)
 	       "                      preserve context)\n"
 	       "                      to original kernel.\n"
 	       " -s, --kexec-file-syscall Use file based syscall for kexec operation\n"
+	       " -c, --kexec-syscall  Use the kexec_load syscall for for compatibility\n"
+	       "                      with systems that don't support -s (default)\n"
 	       " -d, --debug          Enable debugging to help spot a failure.\n"
 	       " -S, --status         Return 0 if the type (by default crash) is loaded.\n"
 	       "\n"
@@ -1368,6 +1370,9 @@ int main(int argc, char *argv[])
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
 			break;
+		case OPT_KEXEC_SYSCALL:
+			do_kexec_file_syscall = 0;
+			break;
 		case OPT_STATUS:
 			do_status = 1;
 			break;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 26225d2c002a..9fd0355eacd0 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -219,6 +219,7 @@ extern int file_types;
 #define OPT_TYPE		't'
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
+#define OPT_KEXEC_SYSCALL	'c'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -246,11 +247,12 @@ extern int file_types;
 	{ "mem-max",		1, 0, OPT_MEM_MAX }, \
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
+	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:psS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v6 5/5] kexec: Add option to fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is not supported

[Michal Suchanek]

Not all architectures implement KEXEC_FILE_LOAD. However, on some
archiectures KEXEC_FILE_LOAD is required when secure boot is enabled in
locked-down mode. Previously users had to select the KEXEC_FILE_LOAD
syscall with undocumented -s option. However, if they did pass the
option kexec would fail on architectures that do not support it.

So add an -a option that tries KEXEC_FILE_LOAD and when it is not
supported tries KEXEC_LOAD.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: instead of changing the deafult add extra option
v4: actually check -ENOSYS as well
v5: add missing break
v6:
  - add note about ENOTSUPP
  - add description to help text
---
 kexec/kexec.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 kexec/kexec.h |  4 +++-
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 87689311af2f..612c1c2afbe5 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1007,6 +1007,10 @@ void usage(void)
 	       " -s, --kexec-file-syscall Use file based syscall for kexec operation\n"
 	       " -c, --kexec-syscall  Use the kexec_load syscall for for compatibility\n"
 	       "                      with systems that don't support -s (default)\n"
+	       " -a, --kexec-syscall-auto  Use file based syscall for kexec and fall\n"
+	       "                      back to the compatibility syscall when file based\n"
+	       "                      syscall is not supported or the kernel did not\n"
+	       "                      understand the image\n"
 	       " -d, --debug          Enable debugging to help spot a failure.\n"
 	       " -S, --status         Return 0 if the type (by default crash) is loaded.\n"
 	       "\n"
@@ -1245,6 +1249,7 @@ int main(int argc, char *argv[])
 	int do_unload = 0;
 	int do_reuse_initrd = 0;
 	int do_kexec_file_syscall = 0;
+	int do_kexec_fallback = 0;
 	int do_status = 0;
 	void *entry = 0;
 	char *type = 0;
@@ -1369,9 +1374,15 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_KEXEC_FILE_SYSCALL:
 			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 0;
 			break;
 		case OPT_KEXEC_SYSCALL:
 			do_kexec_file_syscall = 0;
+			do_kexec_fallback = 0;
+			break;
+		case OPT_KEXEC_SYSCALL_AUTO:
+			do_kexec_file_syscall = 1;
+			do_kexec_fallback = 1;
 			break;
 		case OPT_STATUS:
 			do_status = 1;
@@ -1438,7 +1449,7 @@ int main(int argc, char *argv[])
 		}
 	}
 	if (do_kexec_file_syscall) {
-		if (do_load_jump_back_helper)
+		if (do_load_jump_back_helper && !do_kexec_fallback)
 			die("--load-jump-back-helper not supported with kexec_file_load\n");
 		if (kexec_flags & KEXEC_PRESERVE_CONTEXT)
 			die("--load-preserve-context not supported with kexec_file_load\n");
@@ -1452,16 +1463,60 @@ int main(int argc, char *argv[])
 		result = k_status(kexec_flags);
 	}
 	if (do_unload) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = kexec_file_unload(kexec_file_flags);
-		else
+			if ((result == -ENOSYS) && do_kexec_fallback)
+				do_kexec_file_syscall = 0;
+		}
+		if (!do_kexec_file_syscall)
 			result = k_unload(kexec_flags);
 	}
 	if (do_load && (result == 0)) {
-		if (do_kexec_file_syscall)
+		if (do_kexec_file_syscall) {
 			result = do_kexec_file_load(fileind, argc, argv,
 						 kexec_file_flags);
-		else
+			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 (!do_kexec_file_syscall)
 			result = my_load(type, fileind, argc, argv,
 						kexec_flags, entry);
 	}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 9fd0355eacd0..d445fbe3e486 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -220,6 +220,7 @@ extern int file_types;
 #define OPT_PANIC		'p'
 #define OPT_KEXEC_FILE_SYSCALL	's'
 #define OPT_KEXEC_SYSCALL	'c'
+#define OPT_KEXEC_SYSCALL_AUTO	'a'
 #define OPT_STATUS		'S'
 #define OPT_MEM_MIN             256
 #define OPT_MEM_MAX             257
@@ -248,11 +249,12 @@ extern int file_types;
 	{ "reuseinitrd",	0, 0, OPT_REUSE_INITRD }, \
 	{ "kexec-file-syscall",	0, 0, OPT_KEXEC_FILE_SYSCALL }, \
 	{ "kexec-syscall",	0, 0, OPT_KEXEC_SYSCALL }, \
+	{ "kexec-syscall-auto",	0, 0, OPT_KEXEC_SYSCALL_AUTO }, \
 	{ "debug",		0, 0, OPT_DEBUG }, \
 	{ "status",		0, 0, OPT_STATUS }, \
 	{ "print-ckr-size",     0, 0, OPT_PRINT_CKR_SIZE }, \
 
-#define KEXEC_OPT_STR "h?vdfxyluet:pscS"
+#define KEXEC_OPT_STR "h?vdfxyluet:pscaS"
 
 extern void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr);
 extern void die(const char *fmt, ...)
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 6/6] kexec: Document -s, -c and -a options in the man page

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v5: document that KEXEC_LOAD may be disabled
v6: document that fallback happens in case the kernel does not
understand the image
---
 kexec/kexec.8 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kexec/kexec.8 b/kexec/kexec.8
index e0131b4ea827..fb8a4c9caa45 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -144,6 +144,26 @@ Load the new kernel for use on panic.
 Specify that the new kernel is of this
 .I type.
 .TP
+.BI \-s\ (\-\-kexec-file-syscall)
+Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively.
+.TP
+.BI \-c\ (\-\-kexec-syscall)
+Specify that the old KEXEC_LOAD syscall should be used exclusively (the default).
+.TP
+.BI \-a\ (\-\-kexec-syscall-auto)
+Try the new KEXEC_FILE_LOAD syscall first and when it is not supported or the
+kernel does not understand the supplied image fall back to the old KEXEC_LOAD
+interface.
+
+There is no one single interface that always works.
+
+KEXEC_FILE_LOAD is required on systems that use locked-down secure boot to
+verify the kernel signature.  KEXEC_LOAD may be also disabled in the kernel
+configuration.
+
+KEXEC_LOAD is required for some kernel image formats and on architectures that
+do not implement KEXEC_FILE_LOAD.
+.TP
 .B \-u\ (\-\-unload)
 Unload the current
 .B kexec
-- 
2.13.6


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v6 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Simon Horman]

Hi again Dave,

would you care to review v6?

On Wed, Mar 28, 2018 at 03:15:14PM +0200, Michal Suchanek wrote:
> When the kernel does not know a syscall number it returns -ENOSYS but
> when kexec does not know a syscall number it returns -1. Return -ENOSYS
> from kexec as well.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index cfd837c1b6bb..ab8cff7fe083 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
>  
>  	if (!is_kexec_file_load_implemented()) {
>  		fprintf(stderr, "syscall kexec_file_load not available.\n");
> -		return -1;
> +		return -ENOSYS;
>  	}
>  
>  	if (argc - fileind <= 0) {
> -- 
> 2.13.6
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v6 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Dave Young]

Hi Simon,

On 03/30/18 at 08:29am, Simon Horman wrote:
> Hi again Dave,
> 
> would you care to review v6?

I have no more comments for v6.  Overall looks good to me.

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v6 1/5] kexec: Return -ENOSYS when kexec does not know how to call KEXEC_FILE_LOAD

[Simon Horman]

On Fri, Mar 30, 2018 at 04:00:58PM +0800, Dave Young wrote:
> Hi Simon,
> 
> On 03/30/18 at 08:29am, Simon Horman wrote:
> > Hi again Dave,
> > 
> > would you care to review v6?
> 
> I have no more comments for v6.  Overall looks good to me.

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v6 3/5] kexec: Do not special-case the -s option

[Petr Tesarik]

On Wed, 28 Mar 2018 15:15:16 +0200
Michal Suchanek <msuchanek@suse.de> wrote:

> It is parsed separately to save a few CPU cycles when setting up other
> options but it just complicates the code. So fold it back and set up all
> flags for both KEXEC_LOAD and KEXEC_FILE_LOAD
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  kexec/kexec.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index b793f31ea501..68ae0594d4a7 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
>[...]
> @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
>  			do_exec = 0;
>  			do_shutdown = 0;
>  			do_sync = 0;
> -			if (do_kexec_file_syscall)
> -				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> -			else
> -				kexec_flags = KEXEC_ON_CRASH;
> -			break;

Argh. This break was rather important. "kexec -p" now segfaults on
me, as it falls through to parsing non-existent optarg. :-(

Petr T

> +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> +			kexec_flags = KEXEC_ON_CRASH;
>  		case OPT_MEM_MIN:
>  			mem_min = strtoul(optarg, &endptr, 0);
>  			if (*endptr) {
> @@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
>  			do_reuse_initrd = 1;
>  			break;
>  		case OPT_KEXEC_FILE_SYSCALL:
> -			/* We already parsed it. Nothing to do. */
> +			do_kexec_file_syscall = 1;
>  			break;
>  		case OPT_STATUS:
>  			do_status = 1;


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v6 3/5] kexec: Do not special-case the -s option

[Bhupesh Sharma]

On 04/05/2018 04:35 PM, Petr Tesarik wrote:
> On Wed, 28 Mar 2018 15:15:16 +0200
> Michal Suchanek <msuchanek@suse.de> wrote:
> 
>> It is parsed separately to save a few CPU cycles when setting up other
>> options but it just complicates the code. So fold it back and set up all
>> flags for both KEXEC_LOAD and KEXEC_FILE_LOAD
>>
>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>> ---
>>   kexec/kexec.c | 25 ++++---------------------
>>   1 file changed, 4 insertions(+), 21 deletions(-)
>>
>> diff --git a/kexec/kexec.c b/kexec/kexec.c
>> index b793f31ea501..68ae0594d4a7 100644
>> --- a/kexec/kexec.c
>> +++ b/kexec/kexec.c
>> [...]
>> @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
>>   			do_exec = 0;
>>   			do_shutdown = 0;
>>   			do_sync = 0;
>> -			if (do_kexec_file_syscall)
>> -				kexec_file_flags |= KEXEC_FILE_ON_CRASH;
>> -			else
>> -				kexec_flags = KEXEC_ON_CRASH;
>> -			break;
> 
> Argh. This break was rather important. "kexec -p" now segfaults on
> me, as it falls through to parsing non-existent optarg. :-(
> 

Sigh. Seems 'kexec -p' was never tested with properly with this 
patchset. This is one of the problems I see with this patchset, so we 
definitely would like to get rid of this segmentation fault.

Petr, I am going to share my Tested-by for the patch you shared for 
fixing this issue in the separate thread.

I would request the fix to be picked up earlier into kexec-tools as 
'kexec -p' is currently broken in upstream kexec-tools.

Regards,
Bhupesh


> 
>> +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
>> +			kexec_flags = KEXEC_ON_CRASH;
>>   		case OPT_MEM_MIN:
>>   			mem_min = strtoul(optarg, &endptr, 0);
>>   			if (*endptr) {
>> @@ -1383,7 +1366,7 @@ int main(int argc, char *argv[])
>>   			do_reuse_initrd = 1;
>>   			break;
>>   		case OPT_KEXEC_FILE_SYSCALL:
>> -			/* We already parsed it. Nothing to do. */
>> +			do_kexec_file_syscall = 1;
>>   			break;
>>   		case OPT_STATUS:
>>   			do_status = 1;
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec