[PATCH 0/2] kernel probe function fixes

[PATCH 0/2] kernel probe function fixes

[Dave Young]

During my testing for arm zImage, in 2nd kernel the atags pointer and the
machine_id are not valid, I did a lot of debugging in kernel, finally I found
this is caused by a kexec tools bug instead.

It is a regression since bf06cf2095. It changes uImage probe return values as
below:
 *  0		- valid uImage type
 * -1           - If the image is corrupted.
 *  1           - If the image is not a uImage.

Because uImage will be probed before zImage, but the uImage probe return 1 for
a zImage thus kexec will mistakenly think it is ok to use as uImage.

IMHO, return -1 in case it's not an uImage is better instead of 1 like before
It's not really necessary to introduce a new return value. It might be better
for a new patch to return either 0 or -1 for uImage probe.

But I did not fix the uImage return value yet. In this patchset I did below:
 - ppc64 zImage: return 0 instead of 1 in case of success
 - strictly checking if probe return 0 in kexec.c

Tested arm zImage with use_atags only.

Dave Young (2):
  zImage_ppc64_probe: return 0 instead of 1 in case of success
  kernel image probe function return value checking fix

 kexec/arch/ppc64/kexec-zImage-ppc64.c | 5 ++++-
 kexec/kexec.c                         | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
1.8.3.1


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

[PATCH 1/2] zImage_ppc64_probe: return 0 instead of 1 in case of success

[Dave Young]

All other _probe functions return 0 for probing the kernel
image successfully, so there's no reason to return 1 here.

Fix it to return 0 instead.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 kexec/arch/ppc64/kexec-zImage-ppc64.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kexec/arch/ppc64/kexec-zImage-ppc64.c b/kexec/arch/ppc64/kexec-zImage-ppc64.c
index 24a87c6..d084ee5 100644
--- a/kexec/arch/ppc64/kexec-zImage-ppc64.c
+++ b/kexec/arch/ppc64/kexec-zImage-ppc64.c
@@ -36,6 +36,7 @@
 int zImage_ppc64_probe(FILE *file)
 {
 	Elf32_Ehdr elf;
+	int valid;
 
 	if (fseek(file, 0, SEEK_SET) < 0) {
 		fprintf(stderr, "seek error: %s\n",
@@ -53,7 +54,7 @@ int zImage_ppc64_probe(FILE *file)
 		return -1;
 	}
 
-	return (elf.e_ident[EI_MAG0]  == ELFMAG0        &&
+	valid = (elf.e_ident[EI_MAG0]  == ELFMAG0        &&
 		elf.e_ident[EI_MAG1]  == ELFMAG1        &&
 		elf.e_ident[EI_MAG2]  == ELFMAG2        &&
 		elf.e_ident[EI_MAG3]  == ELFMAG3        &&
@@ -61,6 +62,8 @@ int zImage_ppc64_probe(FILE *file)
 		elf.e_ident[EI_DATA]  == ELFDATA2MSB &&
 		elf.e_type            == ET_EXEC        &&
 		elf.e_machine         == EM_PPC);
+
+	return valid ? 0 : -1;
 }
 
 int zImage_ppc64_load(FILE *file, int UNUSED(argc), char **UNUSED(argv),
-- 
1.8.3.1


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

[PATCH 2/2] kernel image probe function return value checking fix

[Dave Young]

Currently kexec will use the kernel image type when probe function return
value >=0. It looks odd, but previously it works. Since commit bf06cf2095
it does not work anymore.

During my testing for arm zImage, in 2nd kernel the atags pointer and the
machine_id are not valid, I did a lot of debugging in kernel, finally I found
this is caused by a kexec tools bug instead.

Because uImage will be probed before zImage, also the uImage probe return 1
instead of -1 since bf06cf2095, thus kexec will mistakenly think it is uImage.

Fix this issue by regarding it's valid only when probe return 0.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 kexec/kexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index f13e512..703d524 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -691,7 +691,7 @@ static int my_load(const char *type, int fileind, int argc, char **argv,
 	}
 	if (!type || guess_only) {
 		for (i = 0; i < file_types; i++) {
-			if (file_type[i].probe(kernel_buf, kernel_size) >= 0)
+			if (file_type[i].probe(kernel_buf, kernel_size) == 0)
 				break;
 		}
 		if (i == file_types) {
-- 
1.8.3.1


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

Re: [PATCH 1/2] zImage_ppc64_probe: return 0 instead of 1 in case of success

[Simon Horman]

On Thu, Feb 06, 2014 at 02:30:43PM +0800, Dave Young wrote:
> All other _probe functions return 0 for probing the kernel
> image successfully, so there's no reason to return 1 here.
> 
> Fix it to return 0 instead.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>

Thanks, applied.

> ---
>  kexec/arch/ppc64/kexec-zImage-ppc64.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kexec/arch/ppc64/kexec-zImage-ppc64.c b/kexec/arch/ppc64/kexec-zImage-ppc64.c
> index 24a87c6..d084ee5 100644
> --- a/kexec/arch/ppc64/kexec-zImage-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-zImage-ppc64.c
> @@ -36,6 +36,7 @@
>  int zImage_ppc64_probe(FILE *file)
>  {
>  	Elf32_Ehdr elf;
> +	int valid;
>  
>  	if (fseek(file, 0, SEEK_SET) < 0) {
>  		fprintf(stderr, "seek error: %s\n",
> @@ -53,7 +54,7 @@ int zImage_ppc64_probe(FILE *file)
>  		return -1;
>  	}
>  
> -	return (elf.e_ident[EI_MAG0]  == ELFMAG0        &&
> +	valid = (elf.e_ident[EI_MAG0]  == ELFMAG0        &&
>  		elf.e_ident[EI_MAG1]  == ELFMAG1        &&
>  		elf.e_ident[EI_MAG2]  == ELFMAG2        &&
>  		elf.e_ident[EI_MAG3]  == ELFMAG3        &&
> @@ -61,6 +62,8 @@ int zImage_ppc64_probe(FILE *file)
>  		elf.e_ident[EI_DATA]  == ELFDATA2MSB &&
>  		elf.e_type            == ET_EXEC        &&
>  		elf.e_machine         == EM_PPC);
> +
> +	return valid ? 0 : -1;
>  }
>  
>  int zImage_ppc64_load(FILE *file, int UNUSED(argc), char **UNUSED(argv),
> -- 
> 1.8.3.1
> 

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

Re: [PATCH 2/2] kernel image probe function return value checking fix

[Simon Horman]

On Thu, Feb 06, 2014 at 02:30:44PM +0800, Dave Young wrote:
> Currently kexec will use the kernel image type when probe function return
> value >=0. It looks odd, but previously it works. Since commit bf06cf2095
> it does not work anymore.
> 
> During my testing for arm zImage, in 2nd kernel the atags pointer and the
> machine_id are not valid, I did a lot of debugging in kernel, finally I found
> this is caused by a kexec tools bug instead.
> 
> Because uImage will be probed before zImage, also the uImage probe return 1
> instead of -1 since bf06cf2095, thus kexec will mistakenly think it is uImage.
> 
> Fix this issue by regarding it's valid only when probe return 0.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>

Thanks, applied.

> ---
>  kexec/kexec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f13e512..703d524 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -691,7 +691,7 @@ static int my_load(const char *type, int fileind, int argc, char **argv,
>  	}
>  	if (!type || guess_only) {
>  		for (i = 0; i < file_types; i++) {
> -			if (file_type[i].probe(kernel_buf, kernel_size) >= 0)
> +			if (file_type[i].probe(kernel_buf, kernel_size) == 0)
>  				break;
>  		}
>  		if (i == file_types) {
> -- 
> 1.8.3.1
> 

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