On Sun, Apr 28, 2019 at 4:37 PM Dave Young [off-list ref] wrote:

On 04/25/19 at 04:20pm, Pingfan Liu wrote:

quoted

On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger [off-list ref] wrote:

quoted

[...]

quoted

quoted

@@ -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