From: Oliver Neukum <oneukum@suse.com>
To: Ben Hutchings <ben@decadent.org.uk>, mchehab@kernel.org
Cc: Anton Vasilyev <vasilyev@ispras.ru>, linux-media@vger.kernel.org
Subject: Re: [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit
Date: Wed, 21 Aug 2019 10:18:20 +0200 [thread overview]
Message-ID: <1566375500.2611.6.camel@suse.com> (raw)
In-Reply-To: <fe983331d14442a96db3f71066ca0488a8921840.camel@decadent.org.uk>
Am Dienstag, den 20.08.2019, 19:55 +0100 schrieb Ben Hutchings:
> On Tue, 2019-04-30 at 15:07 +0200, Oliver Neukum wrote:
> > dvb_usb_device_exit() frees and uses the device name in that order
> > Fix by storing the name in a buffer before freeing it
> >
> > v2: fixed style issues
> > v3: strscpy used and variable names changed
> > v4: really use strscpy everywhere
> >
> > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > Reported-by: syzbot+26ec41e9f788b3eba396@syzkaller.appspotmail.com
>
> This doesn't fix that bug (and I don't think it fixes a bug at all).
> The name string is static and doesn't get freed until the module it's
> in is freed.
I see.
> Look again at the stack traces in
> <https://syzkaller.appspot.com/bug?extid=26ec41e9f788b3eba396>:
>
> > Allocated by task 21:
>
> [...]
> > kmemdup+0x23/0x50 mm/util.c:118
> >
>
> kmemdup include/linux/string.h:428 [inline]
> > dw2102_probe+0x62c/0xc50
>
> drivers/media/usb/dvb-usb/dw2102.c:2375
> [...]
> > Freed by task 21:
>
> [...]
> >
>
> kfree+0xce/0x290 mm/slub.c:3958
> > dw2102_probe+0x876/0xc50
>
> drivers/media/usb/dvb-usb/dw2102.c:2409
>
> So, d->desc was freed during probe, and is a dangling pointer before
> dvb_usb_device_exit() runs at all.
In that case KASAN would have reported a double free in testing, which
it did not.
> The bug seems to have been introduced by:
>
> commit 299c7007e93645067e1d2743f4e50156de78c4ff
> Author: Anton Vasilyev <vasilyev@ispras.ru>
> Date: Mon Jul 23 13:04:54 2018 -0400
>
> media: dw2102: Fix memleak on sequence of probes
AFAICT this patch only does anything if probe() succeeds, which it does
not. Something is strange.
Regards
Oliver
next prev parent reply other threads:[~2019-08-21 8:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-30 13:07 [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit Oliver Neukum
2019-08-20 18:55 ` Ben Hutchings
2019-08-21 8:18 ` Oliver Neukum [this message]
2019-08-21 12:47 ` Ben Hutchings
2019-08-22 10:41 ` [PATCH] media: dw2102: Fix use after free Anton Vasilyev
2019-08-29 16:56 ` Kees Cook
2020-04-14 10:10 ` Sean Young
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1566375500.2611.6.camel@suse.com \
--to=oneukum@suse.com \
--cc=ben@decadent.org.uk \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=vasilyev@ispras.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.