Message ID | 20230714074011.20989-1-qiang.zhang1211@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a6b2:0:b0:3e4:2afc:c1 with SMTP id c18csp2340692vqm; Fri, 14 Jul 2023 00:53:33 -0700 (PDT) X-Google-Smtp-Source: APBJJlHHcKouZX/ezGiWMmXSSPnC5ahXBDY6ZBccnE8hBp2fSr+SXC+IlJ7xJe4lOAq/2YR8/R3F X-Received: by 2002:a17:906:5182:b0:991:fd87:c6fd with SMTP id y2-20020a170906518200b00991fd87c6fdmr3535818ejk.23.1689321212829; Fri, 14 Jul 2023 00:53:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689321212; cv=none; d=google.com; s=arc-20160816; b=DNqGCYrKQGeG98NVLFwLKXSidSyjtLcl7B3a4Ir7/gwsWG4mqRB7hocaiCtEeUocMX VtgZ5g6Cs9xo2XFHfknRvyZbkEn6u7oC6Kf+OxAXC1TKDcpQdhy1IMtLrTXRM1R+9sU1 ARoToyke3Su5e3lE3bmB/pDE5RnzjaVOs48V/LAcz+NEn4BNwg9GFnlNp6NRmV0mjYXd oxb+KjIafjm7IAH4YmbF2bJcwnzerjmhwkls1DMi75JFF2ly/gL7NjhDlok9NPy+l8Lw 7YiVXecy45vmCh+G63fIkASa16h8zuL7u43cFn2PM6/8xyDp86unljUPXhR3I23Avt3v XvZA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=U6ZgXRAxHZ9WXrqUR0OWVukrZRS3EcS4g4Ru3FHXwWU=; fh=MugexihR7ACxpBraGuwE2k/Z+79NtknPSzuSUf6UGTE=; b=ufj5I6mUy/JuiO9uTvrnZ7apzQpWrSytiVDCiQ1cvMrgZfPURSLPHQ/7ukUZNYCXa1 sASs7BOTVgXucDItOEJrlvdTCUv51VY0gU0cGYbldEn6vRVKyiISmzHGGpz2ahecw9D8 RPClBO0deUgAEYwacpAY2SVnzY6zfI4k9LXM8S+mfzEeMIiTe0yM2purmcpq8eauC2Ft RCwl0/DUtTkbB158VlYjk0F6/BfGpMd6p4o77Ulx/1+QKI4x92t3A+ikb5tQZdo3Icbi P1hLxDVEsC5uQA5hZl+YXBZPvniH1ZLnSnr1vUqJkppttgn09q6hwA7rZq1Hp7DHjWmC dteg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=WjhlPjIg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ga12-20020a170906b84c00b0098debb6fa67si8270178ejb.495.2023.07.14.00.53.00; Fri, 14 Jul 2023 00:53:32 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=WjhlPjIg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234829AbjGNHkW (ORCPT <rfc822;tebrre53rla2o@gmail.com> + 99 others); Fri, 14 Jul 2023 03:40:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38400 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232656AbjGNHkU (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 14 Jul 2023 03:40:20 -0400 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF4ED2D75; Fri, 14 Jul 2023 00:40:19 -0700 (PDT) Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-666e64e97e2so1091061b3a.1; Fri, 14 Jul 2023 00:40:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689320419; x=1691912419; h=message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=U6ZgXRAxHZ9WXrqUR0OWVukrZRS3EcS4g4Ru3FHXwWU=; b=WjhlPjIgw59KQBvZoYx8J85XUYtpKdoK23LxK3g+Cl/Ft4iqUBh9UnXdvcGE8O3K+o PpQGNYWv3D457Px1B+KiESQYc4Bi7gwKQbF8TWuRCYiiaV+RKmZnHHctgyovExJ4JHLW dXubtfLeDryKGhhlgJlMIdmjPcDfqdbcGVWXkTrnvVx7gi8GqNL0UukcYIUDrwM+zAVd EZlRP/k5ZV6PwSV3rDJWrJR5nTqWy9W0v3wiW2iAwEPfTi8L3T0v+VJToizdCGlgWfYk +YHkubKJHB53rcVc71KqkpwPcf9zR4vEJ6Jd4naCmTbll83PtninrjwlJYpI7ycRX+4t gZww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689320419; x=1691912419; h=message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=U6ZgXRAxHZ9WXrqUR0OWVukrZRS3EcS4g4Ru3FHXwWU=; b=SENFsTs1tPCKbU7u3W9IWnR0qBjSZ5PoTWM0v3oqaUUphDQ/Wz8q4/3TKt+u0L3h9O Tbylw8NFOxbOPARqJFiff7rweN5PN3GY6nJ6K7cZ6hpl6Rs6voCPHGEZ5FTI1AyYDPZ1 bxSzb0GUTnnTeR+4xob/8r5PMJ9+e1FNujIY0kFjBqzSQaNUesxLF4i/8ONZQNUPyrHy YSY2/VJHWJ7jEUhBR1QG9ZesMcUXL50I0HGflkWXU/xgaWPeSa3UkCA6MflFnvp9MBy9 4Da8hRaidIlWtY+Pnq9FNO0Ukf9aim/TywyEYcuGpsoMElXQO4K6tnFpM+zYupB111lr 26Aw== X-Gm-Message-State: ABy/qLa9wZ040AIWLVRhJoDZp3SBRlk5L/+N9v/qAZyFIBuEMSkOXPC0 vhKIPtsbcIT2q/EktmXw6Ps/Qg4lsG0= X-Received: by 2002:a05:6a00:c87:b0:641:d9b:a444 with SMTP id a7-20020a056a000c8700b006410d9ba444mr4240602pfv.31.1689320419337; Fri, 14 Jul 2023 00:40:19 -0700 (PDT) Received: from MSCND1355B05.fareast.nevint.com ([183.242.39.186]) by smtp.gmail.com with ESMTPSA id s15-20020a62e70f000000b00682ba300cd1sm6536336pfh.29.2023.07.14.00.40.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Jul 2023 00:40:18 -0700 (PDT) From: Zqiang <qiang.zhang1211@gmail.com> To: andreyknvl@gmail.com, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, qiang.zhang1211@gmail.com Subject: [PATCH] USB: gadget: Fix the memory leak in raw_gadget driver Date: Fri, 14 Jul 2023 15:40:11 +0800 Message-Id: <20230714074011.20989-1-qiang.zhang1211@gmail.com> X-Mailer: git-send-email 2.17.1 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771381680090464274 X-GMAIL-MSGID: 1771381680090464274 |
Series |
USB: gadget: Fix the memory leak in raw_gadget driver
|
|
Commit Message
Z qiang
July 14, 2023, 7:40 a.m. UTC
Currently, increasing raw_dev->count happens before invoke the
raw_queue_event(), if the raw_queue_event() return error, invoke
raw_release() will not trigger the dev_free() to be called.
[ 268.905865][ T5067] raw-gadget.0 gadget.0: failed to queue event
[ 268.912053][ T5067] udc dummy_udc.0: failed to start USB Raw Gadget: -12
[ 268.918885][ T5067] raw-gadget.0: probe of gadget.0 failed with error -12
[ 268.925956][ T5067] UDC core: USB Raw Gadget: couldn't find an available UDC or it's busy
[ 268.934657][ T5067] misc raw-gadget: fail, usb_gadget_register_driver returned -16
BUG: memory leak
[<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076
[<ffffffff8347eb55>] kmalloc include/linux/slab.h:582 [inline]
[<ffffffff8347eb55>] kzalloc include/linux/slab.h:703 [inline]
[<ffffffff8347eb55>] dev_new drivers/usb/gadget/legacy/raw_gadget.c:191 [inline]
[<ffffffff8347eb55>] raw_open+0x45/0x110 drivers/usb/gadget/legacy/raw_gadget.c:385
[<ffffffff827d1d09>] misc_open+0x1a9/0x1f0 drivers/char/misc.c:165
[<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076
[<ffffffff8347cd2f>] kmalloc include/linux/slab.h:582 [inline]
[<ffffffff8347cd2f>] raw_ioctl_init+0xdf/0x410 drivers/usb/gadget/legacy/raw_gadget.c:460
[<ffffffff8347dfe9>] raw_ioctl+0x5f9/0x1120 drivers/usb/gadget/legacy/raw_gadget.c:1250
[<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline]
[<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076
[<ffffffff833ecc6a>] kmalloc include/linux/slab.h:582 [inline]
[<ffffffff833ecc6a>] kzalloc include/linux/slab.h:703 [inline]
[<ffffffff833ecc6a>] dummy_alloc_request+0x5a/0xe0 drivers/usb/gadget/udc/dummy_hcd.c:665
[<ffffffff833e9132>] usb_ep_alloc_request+0x22/0xd0 drivers/usb/gadget/udc/core.c:196
[<ffffffff8347f13d>] gadget_bind+0x6d/0x370 drivers/usb/gadget/legacy/raw_gadget.c:292
This commit therefore invoke kref_get() under the condition that
raw_queue_event() return success.
Reported-by: syzbot+feb045d335c1fdde5bf7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=feb045d335c1fdde5bf7
Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
---
drivers/usb/gadget/legacy/raw_gadget.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Comments
On Fri, Jul 14, 2023 at 9:40 AM Zqiang <qiang.zhang1211@gmail.com> wrote: > > Currently, increasing raw_dev->count happens before invoke the > raw_queue_event(), if the raw_queue_event() return error, invoke > raw_release() will not trigger the dev_free() to be called. > > [ 268.905865][ T5067] raw-gadget.0 gadget.0: failed to queue event > [ 268.912053][ T5067] udc dummy_udc.0: failed to start USB Raw Gadget: -12 > [ 268.918885][ T5067] raw-gadget.0: probe of gadget.0 failed with error -12 > [ 268.925956][ T5067] UDC core: USB Raw Gadget: couldn't find an available UDC or it's busy > [ 268.934657][ T5067] misc raw-gadget: fail, usb_gadget_register_driver returned -16 > > BUG: memory leak > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > [<ffffffff8347eb55>] kmalloc include/linux/slab.h:582 [inline] > [<ffffffff8347eb55>] kzalloc include/linux/slab.h:703 [inline] > [<ffffffff8347eb55>] dev_new drivers/usb/gadget/legacy/raw_gadget.c:191 [inline] > [<ffffffff8347eb55>] raw_open+0x45/0x110 drivers/usb/gadget/legacy/raw_gadget.c:385 > [<ffffffff827d1d09>] misc_open+0x1a9/0x1f0 drivers/char/misc.c:165 > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > [<ffffffff8347cd2f>] kmalloc include/linux/slab.h:582 [inline] > [<ffffffff8347cd2f>] raw_ioctl_init+0xdf/0x410 drivers/usb/gadget/legacy/raw_gadget.c:460 > [<ffffffff8347dfe9>] raw_ioctl+0x5f9/0x1120 drivers/usb/gadget/legacy/raw_gadget.c:1250 > [<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline] > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > [<ffffffff833ecc6a>] kmalloc include/linux/slab.h:582 [inline] > [<ffffffff833ecc6a>] kzalloc include/linux/slab.h:703 [inline] > [<ffffffff833ecc6a>] dummy_alloc_request+0x5a/0xe0 drivers/usb/gadget/udc/dummy_hcd.c:665 > [<ffffffff833e9132>] usb_ep_alloc_request+0x22/0xd0 drivers/usb/gadget/udc/core.c:196 > [<ffffffff8347f13d>] gadget_bind+0x6d/0x370 drivers/usb/gadget/legacy/raw_gadget.c:292 > > This commit therefore invoke kref_get() under the condition that > raw_queue_event() return success. > > Reported-by: syzbot+feb045d335c1fdde5bf7@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=feb045d335c1fdde5bf7 > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > --- > drivers/usb/gadget/legacy/raw_gadget.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c > index 2acece16b890..e549022642e5 100644 > --- a/drivers/usb/gadget/legacy/raw_gadget.c > +++ b/drivers/usb/gadget/legacy/raw_gadget.c > @@ -310,13 +310,15 @@ static int gadget_bind(struct usb_gadget *gadget, > dev->eps_num = i; > spin_unlock_irqrestore(&dev->lock, flags); > > - /* Matches kref_put() in gadget_unbind(). */ > - kref_get(&dev->count); > - > ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL); > - if (ret < 0) > + if (ret < 0) { > dev_err(&gadget->dev, "failed to queue event\n"); > + set_gadget_data(gadget, NULL); > + return ret; > + } > > + /* Matches kref_put() in gadget_unbind(). */ > + kref_get(&dev->count); > return ret; > } > > -- > 2.17.1 Indeed, if gadget_bind fails due to a raw_queue_event failure, the core gadget code will never call gadget_unbind. Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com> Tested-by: Andrey Konovalov <andreyknvl@gmail.com> Thanks!
> > On Fri, Jul 14, 2023 at 9:40 AM Zqiang <qiang.zhang1211@gmail.com> wrote: > > > > Currently, increasing raw_dev->count happens before invoke the > > raw_queue_event(), if the raw_queue_event() return error, invoke > > raw_release() will not trigger the dev_free() to be called. > > > > [ 268.905865][ T5067] raw-gadget.0 gadget.0: failed to queue event > > [ 268.912053][ T5067] udc dummy_udc.0: failed to start USB Raw Gadget: -12 > > [ 268.918885][ T5067] raw-gadget.0: probe of gadget.0 failed with error -12 > > [ 268.925956][ T5067] UDC core: USB Raw Gadget: couldn't find an available UDC or it's busy > > [ 268.934657][ T5067] misc raw-gadget: fail, usb_gadget_register_driver returned -16 > > > > BUG: memory leak > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff8347eb55>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff8347eb55>] kzalloc include/linux/slab.h:703 [inline] > > [<ffffffff8347eb55>] dev_new drivers/usb/gadget/legacy/raw_gadget.c:191 [inline] > > [<ffffffff8347eb55>] raw_open+0x45/0x110 drivers/usb/gadget/legacy/raw_gadget.c:385 > > [<ffffffff827d1d09>] misc_open+0x1a9/0x1f0 drivers/char/misc.c:165 > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff8347cd2f>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff8347cd2f>] raw_ioctl_init+0xdf/0x410 drivers/usb/gadget/legacy/raw_gadget.c:460 > > [<ffffffff8347dfe9>] raw_ioctl+0x5f9/0x1120 drivers/usb/gadget/legacy/raw_gadget.c:1250 > > [<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline] > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff833ecc6a>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff833ecc6a>] kzalloc include/linux/slab.h:703 [inline] > > [<ffffffff833ecc6a>] dummy_alloc_request+0x5a/0xe0 drivers/usb/gadget/udc/dummy_hcd.c:665 > > [<ffffffff833e9132>] usb_ep_alloc_request+0x22/0xd0 drivers/usb/gadget/udc/core.c:196 > > [<ffffffff8347f13d>] gadget_bind+0x6d/0x370 drivers/usb/gadget/legacy/raw_gadget.c:292 > > > > This commit therefore invoke kref_get() under the condition that > > raw_queue_event() return success. > > > > Reported-by: syzbot+feb045d335c1fdde5bf7@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=feb045d335c1fdde5bf7 > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > --- > > drivers/usb/gadget/legacy/raw_gadget.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c > > index 2acece16b890..e549022642e5 100644 > > --- a/drivers/usb/gadget/legacy/raw_gadget.c > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c > > @@ -310,13 +310,15 @@ static int gadget_bind(struct usb_gadget *gadget, > > dev->eps_num = i; > > spin_unlock_irqrestore(&dev->lock, flags); > > > > - /* Matches kref_put() in gadget_unbind(). */ > > - kref_get(&dev->count); > > - > > ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL); > > - if (ret < 0) > > + if (ret < 0) { > > dev_err(&gadget->dev, "failed to queue event\n"); > > + set_gadget_data(gadget, NULL); > > + return ret; > > + } > > > > + /* Matches kref_put() in gadget_unbind(). */ > > + kref_get(&dev->count); > > return ret; > > } > > > > -- > > 2.17.1 > > Indeed, if gadget_bind fails due to a raw_queue_event failure, the > core gadget code will never call gadget_unbind. > > > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com> > Tested-by: Andrey Konovalov <andreyknvl@gmail.com> Thanks for review and test ! > > Thanks!
> > On Fri, Jul 14, 2023 at 9:40 AM Zqiang <qiang.zhang1211@gmail.com> wrote: > > > > Currently, increasing raw_dev->count happens before invoke the > > raw_queue_event(), if the raw_queue_event() return error, invoke > > raw_release() will not trigger the dev_free() to be called. > > > > [ 268.905865][ T5067] raw-gadget.0 gadget.0: failed to queue event > > [ 268.912053][ T5067] udc dummy_udc.0: failed to start USB Raw Gadget: -12 > > [ 268.918885][ T5067] raw-gadget.0: probe of gadget.0 failed with error -12 > > [ 268.925956][ T5067] UDC core: USB Raw Gadget: couldn't find an available UDC or it's busy > > [ 268.934657][ T5067] misc raw-gadget: fail, usb_gadget_register_driver returned -16 > > > > BUG: memory leak > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff8347eb55>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff8347eb55>] kzalloc include/linux/slab.h:703 [inline] > > [<ffffffff8347eb55>] dev_new drivers/usb/gadget/legacy/raw_gadget.c:191 [inline] > > [<ffffffff8347eb55>] raw_open+0x45/0x110 drivers/usb/gadget/legacy/raw_gadget.c:385 > > [<ffffffff827d1d09>] misc_open+0x1a9/0x1f0 drivers/char/misc.c:165 > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff8347cd2f>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff8347cd2f>] raw_ioctl_init+0xdf/0x410 drivers/usb/gadget/legacy/raw_gadget.c:460 > > [<ffffffff8347dfe9>] raw_ioctl+0x5f9/0x1120 drivers/usb/gadget/legacy/raw_gadget.c:1250 > > [<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline] > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > [<ffffffff833ecc6a>] kmalloc include/linux/slab.h:582 [inline] > > [<ffffffff833ecc6a>] kzalloc include/linux/slab.h:703 [inline] > > [<ffffffff833ecc6a>] dummy_alloc_request+0x5a/0xe0 drivers/usb/gadget/udc/dummy_hcd.c:665 > > [<ffffffff833e9132>] usb_ep_alloc_request+0x22/0xd0 drivers/usb/gadget/udc/core.c:196 > > [<ffffffff8347f13d>] gadget_bind+0x6d/0x370 drivers/usb/gadget/legacy/raw_gadget.c:292 > > > > This commit therefore invoke kref_get() under the condition that > > raw_queue_event() return success. > > > > Reported-by: syzbot+feb045d335c1fdde5bf7@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=feb045d335c1fdde5bf7 > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > --- > > drivers/usb/gadget/legacy/raw_gadget.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c > > index 2acece16b890..e549022642e5 100644 > > --- a/drivers/usb/gadget/legacy/raw_gadget.c > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c > > @@ -310,13 +310,15 @@ static int gadget_bind(struct usb_gadget *gadget, > > dev->eps_num = i; > > spin_unlock_irqrestore(&dev->lock, flags); > > > > - /* Matches kref_put() in gadget_unbind(). */ > > - kref_get(&dev->count); > > - > > ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL); > > - if (ret < 0) > > + if (ret < 0) { > > dev_err(&gadget->dev, "failed to queue event\n"); > > + set_gadget_data(gadget, NULL); > > + return ret; > > + } > > > > + /* Matches kref_put() in gadget_unbind(). */ > > + kref_get(&dev->count); > > return ret; > > } > > > > -- > > 2.17.1 > > Indeed, if gadget_bind fails due to a raw_queue_event failure, the > core gadget code will never call gadget_unbind. > > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com> > Tested-by: Andrey Konovalov <andreyknvl@gmail.com> > Hi Greg Friendly ping :) Thanks Zqiang > Thanks!
On Fri, Jul 21, 2023 at 01:56:35PM +0800, Z qiang wrote: > > > > On Fri, Jul 14, 2023 at 9:40 AM Zqiang <qiang.zhang1211@gmail.com> wrote: > > > > > > Currently, increasing raw_dev->count happens before invoke the > > > raw_queue_event(), if the raw_queue_event() return error, invoke > > > raw_release() will not trigger the dev_free() to be called. > > > > > > [ 268.905865][ T5067] raw-gadget.0 gadget.0: failed to queue event > > > [ 268.912053][ T5067] udc dummy_udc.0: failed to start USB Raw Gadget: -12 > > > [ 268.918885][ T5067] raw-gadget.0: probe of gadget.0 failed with error -12 > > > [ 268.925956][ T5067] UDC core: USB Raw Gadget: couldn't find an available UDC or it's busy > > > [ 268.934657][ T5067] misc raw-gadget: fail, usb_gadget_register_driver returned -16 > > > > > > BUG: memory leak > > > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > > [<ffffffff8347eb55>] kmalloc include/linux/slab.h:582 [inline] > > > [<ffffffff8347eb55>] kzalloc include/linux/slab.h:703 [inline] > > > [<ffffffff8347eb55>] dev_new drivers/usb/gadget/legacy/raw_gadget.c:191 [inline] > > > [<ffffffff8347eb55>] raw_open+0x45/0x110 drivers/usb/gadget/legacy/raw_gadget.c:385 > > > [<ffffffff827d1d09>] misc_open+0x1a9/0x1f0 drivers/char/misc.c:165 > > > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > > [<ffffffff8347cd2f>] kmalloc include/linux/slab.h:582 [inline] > > > [<ffffffff8347cd2f>] raw_ioctl_init+0xdf/0x410 drivers/usb/gadget/legacy/raw_gadget.c:460 > > > [<ffffffff8347dfe9>] raw_ioctl+0x5f9/0x1120 drivers/usb/gadget/legacy/raw_gadget.c:1250 > > > [<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline] > > > > > > [<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076 > > > [<ffffffff833ecc6a>] kmalloc include/linux/slab.h:582 [inline] > > > [<ffffffff833ecc6a>] kzalloc include/linux/slab.h:703 [inline] > > > [<ffffffff833ecc6a>] dummy_alloc_request+0x5a/0xe0 drivers/usb/gadget/udc/dummy_hcd.c:665 > > > [<ffffffff833e9132>] usb_ep_alloc_request+0x22/0xd0 drivers/usb/gadget/udc/core.c:196 > > > [<ffffffff8347f13d>] gadget_bind+0x6d/0x370 drivers/usb/gadget/legacy/raw_gadget.c:292 > > > > > > This commit therefore invoke kref_get() under the condition that > > > raw_queue_event() return success. > > > > > > Reported-by: syzbot+feb045d335c1fdde5bf7@syzkaller.appspotmail.com > > > Closes: https://syzkaller.appspot.com/bug?extid=feb045d335c1fdde5bf7 > > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > > --- > > > drivers/usb/gadget/legacy/raw_gadget.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c > > > index 2acece16b890..e549022642e5 100644 > > > --- a/drivers/usb/gadget/legacy/raw_gadget.c > > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c > > > @@ -310,13 +310,15 @@ static int gadget_bind(struct usb_gadget *gadget, > > > dev->eps_num = i; > > > spin_unlock_irqrestore(&dev->lock, flags); > > > > > > - /* Matches kref_put() in gadget_unbind(). */ > > > - kref_get(&dev->count); > > > - > > > ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL); > > > - if (ret < 0) > > > + if (ret < 0) { > > > dev_err(&gadget->dev, "failed to queue event\n"); > > > + set_gadget_data(gadget, NULL); > > > + return ret; > > > + } > > > > > > + /* Matches kref_put() in gadget_unbind(). */ > > > + kref_get(&dev->count); > > > return ret; > > > } > > > > > > -- > > > 2.17.1 > > > > Indeed, if gadget_bind fails due to a raw_queue_event failure, the > > core gadget code will never call gadget_unbind. > > > > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com> > > Tested-by: Andrey Konovalov <andreyknvl@gmail.com> > > > > Hi Greg > > Friendly ping :) $ mdfrm -c ~/mail/todo/ 1685 messages in /home/gregkh/mail/todo/ For various reasons I am way behind in getting to review public patches like these, sorry. Give me a chance to catch up, should be through them all by the end of next week. In the meantime, to help reduce my load, you can always help review other patches that have been sent to the list for review. thanks, greg k-h
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c index 2acece16b890..e549022642e5 100644 --- a/drivers/usb/gadget/legacy/raw_gadget.c +++ b/drivers/usb/gadget/legacy/raw_gadget.c @@ -310,13 +310,15 @@ static int gadget_bind(struct usb_gadget *gadget, dev->eps_num = i; spin_unlock_irqrestore(&dev->lock, flags); - /* Matches kref_put() in gadget_unbind(). */ - kref_get(&dev->count); - ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL); - if (ret < 0) + if (ret < 0) { dev_err(&gadget->dev, "failed to queue event\n"); + set_gadget_data(gadget, NULL); + return ret; + } + /* Matches kref_put() in gadget_unbind(). */ + kref_get(&dev->count); return ret; }