linux-kernel - Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value more indicant

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFgQCTstd667wP6g+maxYekz4u3iBR2R=FHUiS1V=XxTs6MKUw@mail.gmail.com>
Date:   Thu, 25 Apr 2019 16:20:57 +0800
From:   Pingfan Liu <kernelfans@...il.com>
To:     Matthias Brugger <mbrugger@...e.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, Rich Felker <dalias@...c.org>,
        linux-ia64@...r.kernel.org,
        Julien Thierry <julien.thierry@....com>,
        Yangtao Li <tiny.windzz@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Palmer Dabbelt <palmer@...ive.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>, x86@...nel.org,
        linux-mips@...r.kernel.org, Paul Mackerras <paulus@...ba.org>,
        "H. Peter Anvin" <hpa@...or.com>, linux-s390@...r.kernel.org,
        Florian Fainelli <f.fainelli@...il.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        linux-sh@...r.kernel.org, Michael Ellerman <mpe@...erman.id.au>,
        David Hildenbrand <david@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        Ingo Molnar <mingo@...hat.com>,
        linux-arm-kernel@...ts.infradead.org,
        Catalin Marinas <catalin.marinas@....com>,
        James Hogan <jhogan@...nel.org>,
        Dave Young <dyoung@...hat.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Will Deacon <will.deacon@....com>,
        linuxppc-dev@...ts.ozlabs.org,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Borislav Petkov <bp@...en8.de>, Stefan Agner <stefan@...er.ch>,
        Thomas Gleixner <tglx@...utronix.de>,
        Hari Bathini <hbathini@...ux.ibm.com>,
        Jens Axboe <axboe@...nel.dk>, Tony Luck <tony.luck@...el.com>,
        Baoquan He <bhe@...hat.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Robin Murphy <robin.murphy@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Thomas Bogendoerfer <tbogendoerfer@...e.de>,
        Paul Burton <paul.burton@...s.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Logan Gunthorpe <logang@...tatee.com>,
        Greg Hackmann <ghackmann@...roid.com>
Subject: Re: [PATCHv2] kernel/crash: make parse_crashkernel()'s return value
 more indicant

On Wed, Apr 24, 2019 at 4:31 PM Matthias Brugger <mbrugger@...e.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,
> >