linux-kernel - Re: [PATCH 3/3] init: use do_mount() instead of ksys

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMuHMdUF1jXizp3Tz46YDJnzzmJDG_vE3xf-0o+OJiKx-waShw@mail.gmail.com>
Date:   Mon, 16 Dec 2019 14:21:55 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] init: use do_mount() instead of ksys_mount()

Hi all,

On Mon, Dec 16, 2019 at 6:13 AM Eric Biggers <ebiggers@...nel.org> wrote:
> On Sun, Dec 15, 2019 at 07:50:23PM -0800, Linus Torvalds wrote:
> > On Sun, Dec 15, 2019 at 5:35 PM Ondřej Jirman <megi@....cz> wrote:
> > > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> >
> > Duh. So much for the trivial obvious conversion.
> >
> > It didn't take "data might be NULL" into account.
> >
> > A patch like this, perhaps? Untested..

> > --- a/init/do_mounts.c
> > +++ b/init/do_mounts.c
> > @@ -391,17 +391,19 @@ static int __init do_mount_root(const char *name, const char *fs,
> >                                const int flags, const void *data)
> >  {
> >       struct super_block *s;
> > -     char *data_page;
> > -     struct page *p;
> > +     struct page *p = NULL;
> > +     char *data_page = NULL;
> >       int ret;
> >
> > -     /* do_mount() requires a full page as fifth argument */
> > -     p = alloc_page(GFP_KERNEL);
> > -     if (!p)
> > -             return -ENOMEM;
> > -
> > -     data_page = page_address(p);
> > -     strncpy(data_page, data, PAGE_SIZE - 1);
> > +     if (data) {
> > +             /* do_mount() requires a full page as fifth argument */
> > +             p = alloc_page(GFP_KERNEL);
> > +             if (!p)
> > +                     return -ENOMEM;
> > +             data_page = page_address(p);
> > +             strncpy(data_page, data, PAGE_SIZE - 1);
> > +             data_page[PAGE_SIZE - 1] = '\0';
> > +     }
> >
> >       ret = do_mount(name, "/root", fs, flags, data_page);
> >       if (ret)
> > @@ -417,7 +419,8 @@ static int __init do_mount_root(const char *name, const char *fs,
> >              MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
> >
> >  out:
> > -     put_page(p);
> > +     if (p)
> > +             put_page(p);
> >       return ret;
>
> I'm seeing the boot crash too, and Linus' patch fixes it.

#metoo ;-)

Tested-by: Geert Uytterhoeven <geert@...ux-m68k.org>

> Note that adding the line
>
>                 data_page[PAGE_SIZE - 1] = '\0';
>
> is not necessary since do_mount() already does it.

Indeed. So the "-1" can be dropped from the strncpy() call, which brings
it even more in-line with the "full page" comment.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds