netdev - Re: BUG: unable to handle kernel paging request in slhc

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]

Date:   Sat, 16 Mar 2019 10:24:10 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     syzbot <syzbot+6c5d567447bfa30f78e2@...kaller.appspotmail.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>, syzkaller-bugs@...glegroups.com,
        tejaswit@...eaurora.org
Subject: Re: BUG: unable to handle kernel paging request in slhc_free

On Fri, Dec 28, 2018 at 6:41 PM syzbot
<syzbot+6c5d567447bfa30f78e2@...kaller.appspotmail.com> wrote:
>
> Reported-by: syzbot+6c5d567447bfa30f78e2@...kaller.appspotmail.com
>
> BUG: unable to handle kernel paging request at fffffffffffffff4
> RIP: slhc_free+0x30/0xb0 drivers/net/slip/slhc.c:159
> Call Trace:
>   sl_alloc_bufs drivers/net/slip/slip.c:196 [inline]
>   slip_open+0xdee/0x1107 drivers/net/slip/slip.c:821

The error handling in sl_alloc_bufs() is broken.

It does

        slcomp = slhc_init(16, 16);
        if (IS_ERR(slcomp))
                goto err_exit;

and knows that the error case returns an error pointer, but then the
'err_exit:' code just does

        slhc_free(slcomp);

which doesn't handle error pointers.

The slhc code in general is pretty odd, presumably for some legacy
reason. It does things like

        if ( comp == NULLSLCOMPR )
                return;

to compare against NULL. That's some crazy stuff.

I don't think anybody really wants to bother with slip any more, and
the simplest fix seems to just be to let slhc_free() handle all the
error pointers that slhc_init() can return, and do something  like
this:

   drivers/net/slip/slhc.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

  diff --git a/drivers/net/slip/slhc.c b/drivers/net/slip/slhc.c
  index f4e93f5fc204..3ee19a5b03a1 100644
  --- a/drivers/net/slip/slhc.c
  +++ b/drivers/net/slip/slhc.c
  @@ -153,7 +153,7 @@ slhc_init(int rslots, int tslots)
   void
   slhc_free(struct slcompress *comp)
   {
  -       if ( comp == NULLSLCOMPR )
  +       if (IS_ERR_OR_NULL(comp))
                  return;

          if ( comp->tstate != NULLSLSTATE )

which is obviously and intentionally whitespace-damaged, but you get the idea.

David?

                  Linus