From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05A67C3A59E for ; Wed, 21 Aug 2019 08:33:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CAEE82339E for ; Wed, 21 Aug 2019 08:33:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727481AbfHUIdF (ORCPT ); Wed, 21 Aug 2019 04:33:05 -0400 Received: from mx2.suse.de ([195.135.220.15]:50264 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727409AbfHUIdF (ORCPT ); Wed, 21 Aug 2019 04:33:05 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 2A4E3AFB7; Wed, 21 Aug 2019 08:33:04 +0000 (UTC) Message-ID: <1566375500.2611.6.camel@suse.com> Subject: Re: [PATCH] dvb: usb: fix use after free in dvb_usb_device_exit From: Oliver Neukum To: Ben Hutchings , mchehab@kernel.org Cc: Anton Vasilyev , linux-media@vger.kernel.org Date: Wed, 21 Aug 2019 10:18:20 +0200 In-Reply-To: References: <20190430130736.9191-1-oneukum@suse.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.6 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org 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 > > 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 > : > > > 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 > 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