* [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
@ 2019-04-24 6:33 Pingfan Liu
2019-04-24 8:31 ` Matthias Brugger
0 siblings, 1 reply; 9+ messages in thread
From: Pingfan Liu @ 2019-04-24 6:33 UTC (permalink / raw)
To: linux-kernel
Cc: Pingfan Liu, Russell King, Catalin Marinas, Will Deacon,
Tony Luck, Fenghua Yu, Ralf Baechle, Paul Burton, James Hogan,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Yoshinori Sato, Rich Felker,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
Andrew Morton, Julien Thierry, Palmer Dabbelt, Ard Biesheuvel,
Florian Fainelli, Logan Gunthorpe, Robin Murphy, Greg Hackmann,
Stefan Agner, Johannes Weiner, David Hildenbrand, Jens Axboe,
Thomas Bogendoerfer, Greg Kroah-Hartman, Hari Bathini,
Ananth N Mavinakayanahalli, Yangtao Li, Dave Young, Baoquan He,
x86, linux-arm-kernel, linux-ia64, linux-mips, linuxppc-dev,
linux-s390, linux-sh
At present, both return and crash_size should be checked to guarantee the
success of parse_crashkernel().
Take a close look at the cases, which causes crash_size=0. Beside syntax
error, three cases cause parsing to get crash_size=0.
-1st. in parse_crashkernel_mem(), the demanded crash size is bigger than
system ram.
-2nd. in parse_crashkernel_mem(), the system ram size does not match any
item in the range list.
-3rd. "crashkernel=0MB", which is impractical.
All these cases can be treated as invalid argument.
By this way, only need a simple check on return value of
parse_crashkernel().
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Greg Hackmann <ghackmann@android.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Thomas Bogendoerfer <tbogendoerfer@suse.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
---
v1 -> v2: On error, return -EINVAL for all failure cases
arch/arm/kernel/setup.c | 2 +-
arch/arm64/mm/init.c | 2 +-
arch/ia64/kernel/setup.c | 2 +-
arch/mips/kernel/setup.c | 2 +-
arch/powerpc/kernel/fadump.c | 2 +-
arch/powerpc/kernel/machine_kexec.c | 2 +-
arch/s390/kernel/setup.c | 2 +-
arch/sh/kernel/machine_kexec.c | 2 +-
arch/x86/kernel/setup.c | 4 ++--
kernel/crash_core.c | 10 +++++++++-
10 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 5d78b6a..2feab13 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -997,7 +997,7 @@ static void __init reserve_crashkernel(void)
total_mem = get_total_mem();
ret = parse_crashkernel(boot_command_line, total_mem,
&crash_size, &crash_base);
- if (ret)
+ if (ret < 0)
return;
if (crash_base <= 0) {
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 6bc1350..240918c 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -79,7 +79,7 @@ static void __init reserve_crashkernel(void)
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&crash_size, &crash_base);
/* no crashkernel= or invalid value specified */
- if (ret || !crash_size)
+ if (ret < 0)
return;
crash_size = PAGE_ALIGN(crash_size);
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 583a374..3bbb58b 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -277,7 +277,7 @@ static void __init setup_crashkernel(unsigned long total, int *n)
ret = parse_crashkernel(boot_command_line, total,
&size, &base);
- if (ret == 0 && size > 0) {
+ if (!ret) {
if (!base) {
sort_regions(rsvd_region, *n);
*n = merge_regions(rsvd_region, *n);
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 8d1dc6c..168571b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -715,7 +715,7 @@ static void __init mips_parse_crashkernel(void)
total_mem = get_total_mem();
ret = parse_crashkernel(boot_command_line, total_mem,
&crash_size, &crash_base);
- if (ret != 0 || crash_size <= 0)
+ if (ret < 0)
return;
if (!memory_region_available(crash_base, crash_size)) {
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 45a8d0b..3571504 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -376,7 +376,7 @@ static inline unsigned long fadump_calculate_reserve_size(void)
*/
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&size, &base);
- if (ret == 0 && size > 0) {
+ if (!ret) {
unsigned long max_size;
if (fw_dump.reserve_bootvar)
diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index 63f5a93..1697ad2 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -122,7 +122,7 @@ void __init reserve_crashkernel(void)
/* use common parsing */
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&crash_size, &crash_base);
- if (ret == 0 && crash_size > 0) {
+ if (!ret) {
crashk_res.start = crash_base;
crashk_res.end = crash_base + crash_size - 1;
}
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index 2c642af..d4bd61b 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -671,7 +671,7 @@ static void __init reserve_crashkernel(void)
crash_base = ALIGN(crash_base, KEXEC_CRASH_MEM_ALIGN);
crash_size = ALIGN(crash_size, KEXEC_CRASH_MEM_ALIGN);
- if (rc || crash_size == 0)
+ if (rc < 0)
return;
if (memblock.memory.regions[0].size < crash_size) {
diff --git a/arch/sh/kernel/machine_kexec.c b/arch/sh/kernel/machine_kexec.c
index 63d63a3..3c03240 100644
--- a/arch/sh/kernel/machine_kexec.c
+++ b/arch/sh/kernel/machine_kexec.c
@@ -157,7 +157,7 @@ void __init reserve_crashkernel(void)
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&crash_size, &crash_base);
- if (ret == 0 && crash_size > 0) {
+ if (!ret) {
crashk_res.start = crash_base;
crashk_res.end = crash_base + crash_size - 1;
}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3d872a5..592d5ad 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -526,11 +526,11 @@ static void __init reserve_crashkernel(void)
/* crashkernel=XM */
ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base);
- if (ret != 0 || crash_size <= 0) {
+ if (ret < 0) {
/* crashkernel=X,high */
ret = parse_crashkernel_high(boot_command_line, total_mem,
&crash_size, &crash_base);
- if (ret != 0 || crash_size <= 0)
+ if (ret < 0)
return;
high = true;
}
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 093c9f9..83ee4a9 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -108,8 +108,10 @@ static int __init parse_crashkernel_mem(char *cmdline,
return -EINVAL;
}
}
- } else
+ } else {
pr_info("crashkernel size resulted in zero bytes\n");
+ return -EINVAL;
+ }
return 0;
}
@@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
pr_warn("crashkernel: unrecognized char: %c\n", *cur);
return -EINVAL;
}
+ if (*crash_size == 0)
+ return -EINVAL;
return 0;
}
@@ -181,6 +185,8 @@ static int __init parse_crashkernel_suffix(char *cmdline,
pr_warn("crashkernel: unrecognized char: %c\n", *cur);
return -EINVAL;
}
+ if (*crash_size == 0)
+ return -EINVAL;
return 0;
}
@@ -266,6 +272,8 @@ static int __init __parse_crashkernel(char *cmdline,
/*
* That function is the entry point for command line parsing and should be
* called from the arch-specific code.
+ * On success 0. On error for either syntax error or crash_size=0, -EINVAL is
+ * returned.
*/
int __init parse_crashkernel(char *cmdline,
unsigned long long system_ram,
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-24 6:33 [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant Pingfan Liu
@ 2019-04-24 8:31 ` Matthias Brugger
2019-04-25 8:20 ` Pingfan Liu
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2019-04-24 8:31 UTC (permalink / raw)
To: Pingfan Liu, linux-kernel
Cc: Rich Felker, linux-ia64, Julien Thierry, Yangtao Li,
Benjamin Herrenschmidt, Palmer Dabbelt, Heiko Carstens, x86,
linux-mips, Paul Mackerras, H. Peter Anvin, linux-s390,
Florian Fainelli, Yoshinori Sato, linux-sh, Michael Ellerman,
David Hildenbrand, Russell King, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, James Hogan, Dave Young, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann
On 24/04/2019 08:33, Pingfan Liu wrote:
> At present, both return and crash_size should be checked to guarantee the
> success of parse_crashkernel().
>
> Take a close look at the cases, which causes crash_size=0. Beside syntax
> error, three cases cause parsing to get crash_size=0.
> -1st. in parse_crashkernel_mem(), the demanded crash size is bigger than
> system ram.
> -2nd. in parse_crashkernel_mem(), the system ram size does not match any
> item in the range list.
> -3rd. "crashkernel=0MB", which is impractical.
>
> All these cases can be treated as invalid argument.
>
> By this way, only need a simple check on return value of
> parse_crashkernel().
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Greg Hackmann <ghackmann@android.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Cc: Yangtao Li <tiny.windzz@gmail.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: x86@kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> ---
> v1 -> v2: On error, return -EINVAL for all failure cases
>
> arch/arm/kernel/setup.c | 2 +-
> arch/arm64/mm/init.c | 2 +-
> arch/ia64/kernel/setup.c | 2 +-
> arch/mips/kernel/setup.c | 2 +-
> arch/powerpc/kernel/fadump.c | 2 +-
> arch/powerpc/kernel/machine_kexec.c | 2 +-
> arch/s390/kernel/setup.c | 2 +-
> arch/sh/kernel/machine_kexec.c | 2 +-
> arch/x86/kernel/setup.c | 4 ++--
> kernel/crash_core.c | 10 +++++++++-
> 10 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 5d78b6a..2feab13 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -997,7 +997,7 @@ static void __init reserve_crashkernel(void)
> total_mem = get_total_mem();
> ret = parse_crashkernel(boot_command_line, total_mem,
> &crash_size, &crash_base);
> - if (ret)
> + if (ret < 0)
> return;
>
> if (crash_base <= 0) {
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 6bc1350..240918c 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -79,7 +79,7 @@ static void __init reserve_crashkernel(void)
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &crash_size, &crash_base);
> /* no crashkernel= or invalid value specified */
> - if (ret || !crash_size)
> + if (ret < 0)
> return;
>
> crash_size = PAGE_ALIGN(crash_size);
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 583a374..3bbb58b 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -277,7 +277,7 @@ static void __init setup_crashkernel(unsigned long total, int *n)
>
> ret = parse_crashkernel(boot_command_line, total,
> &size, &base);
> - if (ret == 0 && size > 0) {
> + if (!ret) {
> if (!base) {
> sort_regions(rsvd_region, *n);
> *n = merge_regions(rsvd_region, *n);
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 8d1dc6c..168571b 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -715,7 +715,7 @@ static void __init mips_parse_crashkernel(void)
> total_mem = get_total_mem();
> ret = parse_crashkernel(boot_command_line, total_mem,
> &crash_size, &crash_base);
> - if (ret != 0 || crash_size <= 0)
> + if (ret < 0)
> return;
>
> if (!memory_region_available(crash_base, crash_size)) {
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 45a8d0b..3571504 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -376,7 +376,7 @@ static inline unsigned long fadump_calculate_reserve_size(void)
> */
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &size, &base);
> - if (ret == 0 && size > 0) {
> + if (!ret) {
> unsigned long max_size;
>
> if (fw_dump.reserve_bootvar)
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index 63f5a93..1697ad2 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -122,7 +122,7 @@ void __init reserve_crashkernel(void)
> /* use common parsing */
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &crash_size, &crash_base);
> - if (ret == 0 && crash_size > 0) {
> + if (!ret) {
> crashk_res.start = crash_base;
> crashk_res.end = crash_base + crash_size - 1;
> }
> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index 2c642af..d4bd61b 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c
> @@ -671,7 +671,7 @@ static void __init reserve_crashkernel(void)
>
> crash_base = ALIGN(crash_base, KEXEC_CRASH_MEM_ALIGN);
> crash_size = ALIGN(crash_size, KEXEC_CRASH_MEM_ALIGN);
> - if (rc || crash_size == 0)
> + if (rc < 0)
> return;
>
> if (memblock.memory.regions[0].size < crash_size) {
> diff --git a/arch/sh/kernel/machine_kexec.c b/arch/sh/kernel/machine_kexec.c
> index 63d63a3..3c03240 100644
> --- a/arch/sh/kernel/machine_kexec.c
> +++ b/arch/sh/kernel/machine_kexec.c
> @@ -157,7 +157,7 @@ void __init reserve_crashkernel(void)
>
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &crash_size, &crash_base);
> - if (ret == 0 && crash_size > 0) {
> + if (!ret) {
> crashk_res.start = crash_base;
> crashk_res.end = crash_base + crash_size - 1;
> }
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 3d872a5..592d5ad 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -526,11 +526,11 @@ static void __init reserve_crashkernel(void)
>
> /* crashkernel=XM */
> ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base);
> - if (ret != 0 || crash_size <= 0) {
> + if (ret < 0) {
> /* crashkernel=X,high */
> ret = parse_crashkernel_high(boot_command_line, total_mem,
> &crash_size, &crash_base);
> - if (ret != 0 || crash_size <= 0)
> + if (ret < 0)
> return;
> high = true;
> }
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 093c9f9..83ee4a9 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -108,8 +108,10 @@ static int __init parse_crashkernel_mem(char *cmdline,
> return -EINVAL;
> }
> }
> - } else
> + } else {
> pr_info("crashkernel size resulted in zero bytes\n");
> + return -EINVAL;
> + }
>
> return 0;
> }
> @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> return -EINVAL;
> }
> + if (*crash_size == 0)
> + return -EINVAL;
This covers the case where I pass an argument like "crashkernel=0M" ?
Can't we fix that by using kstrtoull() in memparse and check if the return value
is < 0? In that case we could return without updating the retptr and we will be
fine.
>
> return 0;
> }
> @@ -181,6 +185,8 @@ static int __init parse_crashkernel_suffix(char *cmdline,
> pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> return -EINVAL;
> }
> + if (*crash_size == 0)
> + return -EINVAL;
Same here.
>
> return 0;
> }
> @@ -266,6 +272,8 @@ static int __init __parse_crashkernel(char *cmdline,
> /*
> * That function is the entry point for command line parsing and should be
> * called from the arch-specific code.
> + * On success 0. On error for either syntax error or crash_size=0, -EINVAL is
> + * returned.
> */
> int __init parse_crashkernel(char *cmdline,
> unsigned long long system_ram,
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-24 8:31 ` Matthias Brugger
@ 2019-04-25 8:20 ` Pingfan Liu
2019-04-28 8:37 ` Dave Young
2019-05-02 6:22 ` Pingfan Liu
0 siblings, 2 replies; 9+ messages in thread
From: Pingfan Liu @ 2019-04-25 8:20 UTC (permalink / raw)
To: Matthias Brugger
Cc: LKML, Rich Felker, linux-ia64, Julien Thierry, Yangtao Li,
Benjamin Herrenschmidt, Palmer Dabbelt, Heiko Carstens, x86,
linux-mips, Paul Mackerras, H. Peter Anvin, linux-s390,
Florian Fainelli, Yoshinori Sato, linux-sh, Michael Ellerman,
David Hildenbrand, Russell King, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, James Hogan, Dave Young, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann
On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
>
>
[...]
> > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > return -EINVAL;
> > }
> > + if (*crash_size == 0)
> > + return -EINVAL;
>
> This covers the case where I pass an argument like "crashkernel=0M" ?
> Can't we fix that by using kstrtoull() in memparse and check if the return value
> is < 0? In that case we could return without updating the retptr and we will be
> fine.
>
It seems that kstrtoull() treats 0M as invalid parameter, while
simple_strtoull() does not.
If changed like your suggestion, then all the callers of memparse()
will treats 0M as invalid parameter. This affects many components
besides kexec. Not sure this can be done or not.
Regards,
Pingfan
> >
> > return 0;
> > }
> > @@ -181,6 +185,8 @@ static int __init parse_crashkernel_suffix(char *cmdline,
> > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > return -EINVAL;
> > }
> > + if (*crash_size == 0)
> > + return -EINVAL;
>
> Same here.
>
> >
> > return 0;
> > }
> > @@ -266,6 +272,8 @@ static int __init __parse_crashkernel(char *cmdline,
> > /*
> > * That function is the entry point for command line parsing and should be
> > * called from the arch-specific code.
> > + * On success 0. On error for either syntax error or crash_size=0, -EINVAL is
> > + * returned.
> > */
> > int __init parse_crashkernel(char *cmdline,
> > unsigned long long system_ram,
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-25 8:20 ` Pingfan Liu
@ 2019-04-28 8:37 ` Dave Young
2019-04-29 3:04 ` Pingfan Liu
2019-05-02 6:22 ` Pingfan Liu
1 sibling, 1 reply; 9+ messages in thread
From: Dave Young @ 2019-04-28 8:37 UTC (permalink / raw)
To: Pingfan Liu
Cc: Matthias Brugger, LKML, Rich Felker, linux-ia64, Julien Thierry,
Yangtao Li, Benjamin Herrenschmidt, Palmer Dabbelt,
Heiko Carstens, x86, linux-mips, Paul Mackerras, H. Peter Anvin,
linux-s390, Florian Fainelli, Yoshinori Sato, linux-sh,
Michael Ellerman, David Hildenbrand, Russell King, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, James Hogan, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann, kexec
On 04/25/19 at 04:20pm, Pingfan Liu wrote:
> On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> >
> >
> [...]
> > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > return -EINVAL;
> > > }
> > > + if (*crash_size == 0)
> > > + return -EINVAL;
> >
> > This covers the case where I pass an argument like "crashkernel=0M" ?
> > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > is < 0? In that case we could return without updating the retptr and we will be
> > fine.
> >
> It seems that kstrtoull() treats 0M as invalid parameter, while
> simple_strtoull() does not.
>
> If changed like your suggestion, then all the callers of memparse()
> will treats 0M as invalid parameter. This affects many components
> besides kexec. Not sure this can be done or not.
simple_strtoull is obsolete, move to kstrtoull is the right way.
$ git grep memparse|wc
158 950 10479
Except some documentation/tools etc there are still a log of callers
which directly use the return value as the ull number without error
checking.
So it would be good to mark memparse as obsolete as well in
lib/cmdline.c, and introduce a new function eg. kmemparse() to use
kstrtoull, and return a real error code, and save the size in an
argument like &size. Then update X86 crashkernel code to use it.
Thanks
Dave
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-28 8:37 ` Dave Young
@ 2019-04-29 3:04 ` Pingfan Liu
2019-04-29 4:48 ` Pingfan Liu
0 siblings, 1 reply; 9+ messages in thread
From: Pingfan Liu @ 2019-04-29 3:04 UTC (permalink / raw)
To: Dave Young
Cc: Matthias Brugger, LKML, Rich Felker, linux-ia64, Julien Thierry,
Yangtao Li, Benjamin Herrenschmidt, Palmer Dabbelt,
Heiko Carstens, x86, linux-mips, Paul Mackerras, H. Peter Anvin,
linux-s390, Florian Fainelli, Yoshinori Sato, linux-sh,
Michael Ellerman, David Hildenbrand, Russell King, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, James Hogan, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann, kexec
On Sun, Apr 28, 2019 at 4:37 PM Dave Young <dyoung@redhat.com> wrote:
>
> On 04/25/19 at 04:20pm, Pingfan Liu wrote:
> > On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> > >
> > >
> > [...]
> > > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > > return -EINVAL;
> > > > }
> > > > + if (*crash_size == 0)
> > > > + return -EINVAL;
> > >
> > > This covers the case where I pass an argument like "crashkernel=0M" ?
> > > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > > is < 0? In that case we could return without updating the retptr and we will be
> > > fine.
> > >
> > It seems that kstrtoull() treats 0M as invalid parameter, while
> > simple_strtoull() does not.
> >
> > If changed like your suggestion, then all the callers of memparse()
> > will treats 0M as invalid parameter. This affects many components
> > besides kexec. Not sure this can be done or not.
>
> simple_strtoull is obsolete, move to kstrtoull is the right way.
>
> $ git grep memparse|wc
> 158 950 10479
>
> Except some documentation/tools etc there are still a log of callers
> which directly use the return value as the ull number without error
> checking.
>
> So it would be good to mark memparse as obsolete as well in
> lib/cmdline.c, and introduce a new function eg. kmemparse() to use
> kstrtoull, and return a real error code, and save the size in an
> argument like &size. Then update X86 crashkernel code to use it.
>
Thank for your good suggestion.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-29 3:04 ` Pingfan Liu
@ 2019-04-29 4:48 ` Pingfan Liu
2019-04-29 5:04 ` Dave Young
0 siblings, 1 reply; 9+ messages in thread
From: Pingfan Liu @ 2019-04-29 4:48 UTC (permalink / raw)
To: Dave Young
Cc: Matthias Brugger, LKML, Rich Felker, linux-ia64, Julien Thierry,
Yangtao Li, Benjamin Herrenschmidt, Palmer Dabbelt,
Heiko Carstens, x86, linux-mips, Paul Mackerras, H. Peter Anvin,
linux-s390, Florian Fainelli, Yoshinori Sato, linux-sh,
Michael Ellerman, David Hildenbrand, Russell King, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, James Hogan, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann, kexec
On Mon, Apr 29, 2019 at 11:04 AM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> On Sun, Apr 28, 2019 at 4:37 PM Dave Young <dyoung@redhat.com> wrote:
> >
> > On 04/25/19 at 04:20pm, Pingfan Liu wrote:
> > > On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> > > >
> > > >
> > > [...]
> > > > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > > > return -EINVAL;
> > > > > }
> > > > > + if (*crash_size == 0)
> > > > > + return -EINVAL;
> > > >
> > > > This covers the case where I pass an argument like "crashkernel=0M" ?
> > > > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > > > is < 0? In that case we could return without updating the retptr and we will be
> > > > fine.
> > > >
> > > It seems that kstrtoull() treats 0M as invalid parameter, while
> > > simple_strtoull() does not.
> > >
> > > If changed like your suggestion, then all the callers of memparse()
> > > will treats 0M as invalid parameter. This affects many components
> > > besides kexec. Not sure this can be done or not.
> >
> > simple_strtoull is obsolete, move to kstrtoull is the right way.
> >
> > $ git grep memparse|wc
> > 158 950 10479
> >
> > Except some documentation/tools etc there are still a log of callers
> > which directly use the return value as the ull number without error
> > checking.
> >
> > So it would be good to mark memparse as obsolete as well in
> > lib/cmdline.c, and introduce a new function eg. kmemparse() to use
> > kstrtoull, and return a real error code, and save the size in an
> > argument like &size. Then update X86 crashkernel code to use it.
> >
> Thank for your good suggestion.
>
Go through the v5.0 kernel code, I think it will be a huge job.
The difference between unsigned long long simple_strtoull(const char
*cp, char **endp, unsigned int base) and int _kstrtoull(const char *s,
unsigned int base, unsigned long long *res) is bigger than expected,
especially the output parameter @res. Many references to
memparse(const char *ptr, char **retptr) rely on @retptr to work. A
typical example from arch/x86/kernel/e820.c
mem_size = memparse(p, &p);
if (p == oldp)
return -EINVAL;
userdef = 1;
if (*p == '@') { <----------- here
start_at = memparse(p+1, &p);
e820__range_add(start_at, mem_size, E820_TYPE_RAM);
} else if (*p == '#') {
start_at = memparse(p+1, &p);
e820__range_add(start_at, mem_size, E820_TYPE_ACPI);
} else if (*p == '$') {
start_at = memparse(p+1, &p);
e820__range_add(start_at, mem_size, E820_TYPE_RESERVED);
}
So we need to resolve the prototype of kstrtoull() firstly, and maybe
kstrtouint() etc too. All of them have lots of references in kernel.
Any idea about this?
Thanks,
Pingfan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-29 4:48 ` Pingfan Liu
@ 2019-04-29 5:04 ` Dave Young
0 siblings, 0 replies; 9+ messages in thread
From: Dave Young @ 2019-04-29 5:04 UTC (permalink / raw)
To: Pingfan Liu
Cc: Matthias Brugger, LKML, Rich Felker, linux-ia64, Julien Thierry,
Yangtao Li, Benjamin Herrenschmidt, Palmer Dabbelt,
Heiko Carstens, x86, linux-mips, Paul Mackerras, H. Peter Anvin,
linux-s390, Florian Fainelli, Yoshinori Sato, linux-sh,
Michael Ellerman, David Hildenbrand, Russell King, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, James Hogan, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann, kexec
On 04/29/19 at 12:48pm, Pingfan Liu wrote:
> On Mon, Apr 29, 2019 at 11:04 AM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > On Sun, Apr 28, 2019 at 4:37 PM Dave Young <dyoung@redhat.com> wrote:
> > >
> > > On 04/25/19 at 04:20pm, Pingfan Liu wrote:
> > > > On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> > > > >
> > > > >
> > > > [...]
> > > > > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > > > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > > > > return -EINVAL;
> > > > > > }
> > > > > > + if (*crash_size == 0)
> > > > > > + return -EINVAL;
> > > > >
> > > > > This covers the case where I pass an argument like "crashkernel=0M" ?
> > > > > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > > > > is < 0? In that case we could return without updating the retptr and we will be
> > > > > fine.
> > > > >
> > > > It seems that kstrtoull() treats 0M as invalid parameter, while
> > > > simple_strtoull() does not.
> > > >
> > > > If changed like your suggestion, then all the callers of memparse()
> > > > will treats 0M as invalid parameter. This affects many components
> > > > besides kexec. Not sure this can be done or not.
> > >
> > > simple_strtoull is obsolete, move to kstrtoull is the right way.
> > >
> > > $ git grep memparse|wc
> > > 158 950 10479
> > >
> > > Except some documentation/tools etc there are still a log of callers
> > > which directly use the return value as the ull number without error
> > > checking.
> > >
> > > So it would be good to mark memparse as obsolete as well in
> > > lib/cmdline.c, and introduce a new function eg. kmemparse() to use
> > > kstrtoull, and return a real error code, and save the size in an
> > > argument like &size. Then update X86 crashkernel code to use it.
> > >
> > Thank for your good suggestion.
> >
> Go through the v5.0 kernel code, I think it will be a huge job.
>
> The difference between unsigned long long simple_strtoull(const char
> *cp, char **endp, unsigned int base) and int _kstrtoull(const char *s,
> unsigned int base, unsigned long long *res) is bigger than expected,
> especially the output parameter @res. Many references to
> memparse(const char *ptr, char **retptr) rely on @retptr to work. A
> typical example from arch/x86/kernel/e820.c
> mem_size = memparse(p, &p);
> if (p == oldp)
> return -EINVAL;
>
> userdef = 1;
> if (*p == '@') { <----------- here
> start_at = memparse(p+1, &p);
> e820__range_add(start_at, mem_size, E820_TYPE_RAM);
> } else if (*p == '#') {
> start_at = memparse(p+1, &p);
> e820__range_add(start_at, mem_size, E820_TYPE_ACPI);
> } else if (*p == '$') {
> start_at = memparse(p+1, &p);
> e820__range_add(start_at, mem_size, E820_TYPE_RESERVED);
> }
>
> So we need to resolve the prototype of kstrtoull() firstly, and maybe
> kstrtouint() etc too. All of them have lots of references in kernel.
>
> Any idea about this?
Not only this place, a lot of other places, I think no hurry to fix them
all at one time.
As we talked just do it according to previous reply, mark memparse as
obsolete, and create a new function to use kstrtoull, and make it used
in crashkernel code first.
Thanks
Dave
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-04-25 8:20 ` Pingfan Liu
2019-04-28 8:37 ` Dave Young
@ 2019-05-02 6:22 ` Pingfan Liu
2019-05-24 3:11 ` Pingfan Liu
1 sibling, 1 reply; 9+ messages in thread
From: Pingfan Liu @ 2019-05-02 6:22 UTC (permalink / raw)
To: Matthias Brugger
Cc: LKML, Rich Felker, linux-ia64, Julien Thierry, Yangtao Li,
Benjamin Herrenschmidt, Palmer Dabbelt, Heiko Carstens, x86,
linux-mips, Paul Mackerras, H. Peter Anvin, linux-s390,
Florian Fainelli, Yoshinori Sato, linux-sh, Michael Ellerman,
David Hildenbrand, Russell King, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, James Hogan, Dave Young, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann
On Thu, Apr 25, 2019 at 4:20 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> >
> >
> [...]
> > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > return -EINVAL;
> > > }
> > > + if (*crash_size == 0)
> > > + return -EINVAL;
> >
> > This covers the case where I pass an argument like "crashkernel=0M" ?
> > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > is < 0? In that case we could return without updating the retptr and we will be
> > fine.
After a series of work, I suddenly realized that it can not be done
like this way. "0M" causes kstrtoull() to return -EINVAL, but this is
caused by "M", not "0". If passing "0" to kstrtoull(), it will return
0 on success.
> >
> It seems that kstrtoull() treats 0M as invalid parameter, while
> simple_strtoull() does not.
>
My careless going through the code. And I tested with a valid value
"256M" using kstrtoull(), it also returned -EINVAL.
So I think there is no way to distinguish 0 from a positive value
inside this basic math function.
Do I miss anything?
Thanks and regards,
Pingfan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant
2019-05-02 6:22 ` Pingfan Liu
@ 2019-05-24 3:11 ` Pingfan Liu
0 siblings, 0 replies; 9+ messages in thread
From: Pingfan Liu @ 2019-05-24 3:11 UTC (permalink / raw)
To: Matthias Brugger
Cc: LKML, Rich Felker, linux-ia64, Julien Thierry, Yangtao Li,
Benjamin Herrenschmidt, Palmer Dabbelt, Heiko Carstens, x86,
linux-mips, Paul Mackerras, H. Peter Anvin, linux-s390,
Florian Fainelli, Yoshinori Sato, linux-sh, Michael Ellerman,
David Hildenbrand, Russell King, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, James Hogan, Dave Young, Fenghua Yu,
Will Deacon, linuxppc-dev, Ananth N Mavinakayanahalli,
Borislav Petkov, Stefan Agner, Thomas Gleixner, Hari Bathini,
Jens Axboe, Tony Luck, Baoquan He, Ard Biesheuvel, Robin Murphy,
Greg Kroah-Hartman, Ralf Baechle, Thomas Bogendoerfer,
Paul Burton, Johannes Weiner, Martin Schwidefsky, Andrew Morton,
Logan Gunthorpe, Greg Hackmann
Matthias, ping? Any suggestions?
Thanks,
Pingfan
On Thu, May 2, 2019 at 2:22 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> On Thu, Apr 25, 2019 at 4:20 PM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@suse.com> wrote:
> > >
> > >
> > [...]
> > > > @@ -139,6 +141,8 @@ static int __init parse_crashkernel_simple(char *cmdline,
> > > > pr_warn("crashkernel: unrecognized char: %c\n", *cur);
> > > > return -EINVAL;
> > > > }
> > > > + if (*crash_size == 0)
> > > > + return -EINVAL;
> > >
> > > This covers the case where I pass an argument like "crashkernel=0M" ?
> > > Can't we fix that by using kstrtoull() in memparse and check if the return value
> > > is < 0? In that case we could return without updating the retptr and we will be
> > > fine.
> After a series of work, I suddenly realized that it can not be done
> like this way. "0M" causes kstrtoull() to return -EINVAL, but this is
> caused by "M", not "0". If passing "0" to kstrtoull(), it will return
> 0 on success.
>
> > >
> > It seems that kstrtoull() treats 0M as invalid parameter, while
> > simple_strtoull() does not.
> >
> My careless going through the code. And I tested with a valid value
> "256M" using kstrtoull(), it also returned -EINVAL.
>
> So I think there is no way to distinguish 0 from a positive value
> inside this basic math function.
> Do I miss anything?
>
> Thanks and regards,
> Pingfan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-24 3:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 6:33 [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant Pingfan Liu
2019-04-24 8:31 ` Matthias Brugger
2019-04-25 8:20 ` Pingfan Liu
2019-04-28 8:37 ` Dave Young
2019-04-29 3:04 ` Pingfan Liu
2019-04-29 4:48 ` Pingfan Liu
2019-04-29 5:04 ` Dave Young
2019-05-02 6:22 ` Pingfan Liu
2019-05-24 3:11 ` Pingfan Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).