Message ID | 20230401200327.16800-1-gregkh@linuxfoundation.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1408894vqo; Sat, 1 Apr 2023 13:08:03 -0700 (PDT) X-Google-Smtp-Source: AKy350Y8+SA+uS0yQ0Omfmxqmiy/mYY6akH/VcjzA/irV2wIhv4XHtYgJ4hI+AEo8KldVg2jmWx9 X-Received: by 2002:aa7:ccce:0:b0:500:5627:a20a with SMTP id y14-20020aa7ccce000000b005005627a20amr27130817edt.25.1680379682953; Sat, 01 Apr 2023 13:08:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680379682; cv=none; d=google.com; s=arc-20160816; b=MolV/eVe7LiXHC1LPbdokjZNZ9Nc5ePBnEB/WmZBjbKCkH8VHUNZzQupF7G4mF+e7P fkULmBJF0Wo2nxTsKT+Gt95CvHEEiyijJP07pm65WWqi37dDssVpMLKmP3o1jF+GYLmG Nt0f6UGs16r0oUR8BIlajZL90GLTkVjSnXwzlTcbQQenCYWrok5Wzycb4BAioHxlrYkl Q/0o+TVNmDV/yD62hcNELn1qb5V0YynoeRM6dF5aZoyA295mh8wgO9+kJExP2UudAdPA cE/fvjO+/ZsgZEOwNDyl0hDPThIL9DqMlNGpnELYhHVtt04jWwziDeANPwgbCJj/HhJ3 FPIA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=r13oAKuja3mghEqJKR4OeDyQF+tS9L5CSpIcUMYV3Mk=; b=M5jhdiZALICJAffRIUbQi9aYtkZsnTfCB464XJ7X1Gpuaz41W69s4nA7ug9k9Ffw6j kgNQsjMiUbv0cqGqaa1V5IybXp8o3IhPclneXgDSjS8wtZdGoxdmIMdb0L3I4f1WBel/ CXK4ifkRx1BOUDUH6XKZGqa2EYWYLgMNvInavkRYfrEjeohsfxHLcDwbUd8Uuq0aUQji QDbnf+3TP4beOpy2b0bmDH5bLa6zn/4FpTMEl7/ZjD8ZyrqTwv/Wq3KZKJVe+oz2widK FhmB3svBib/8TnKcnEb5zvsqMS09/V/pK9ftMoRiHWlZ1mAA4uULpSKzMH/5B7nAdQnC 01/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=zUFq15iA; 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=NONE dis=NONE) header.from=linuxfoundation.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ay25-20020a056402203900b004aaa70a9f13si2203594edb.298.2023.04.01.13.07.38; Sat, 01 Apr 2023 13:08:02 -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=@linuxfoundation.org header.s=korg header.b=zUFq15iA; 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=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229711AbjDAUDi (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Sat, 1 Apr 2023 16:03:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229450AbjDAUDh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 1 Apr 2023 16:03:37 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D864AB761; Sat, 1 Apr 2023 13:03:35 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 99C34B801BE; Sat, 1 Apr 2023 20:03:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E9301C433D2; Sat, 1 Apr 2023 20:03:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1680379413; bh=/Fygv1Pu1n1/p5GSibbFK63D+X3dR2C/FWH95CEqEGk=; h=From:To:Cc:Subject:Date:From; b=zUFq15iAC6ea58bjO5m2qy/Sv5UBX72zObbg4TVIcVZtUI+WapR6+rYY4XQNi+ALR CBfsHTjAIr9PLl1Ged4hNRgJPVN1A/oWdba63NMFZqIL+SgJCTeZI/xrVbETkXrhc9 mfwuxLAHL9VmiDzNdpq+CbIQ6sZ+CpMprbceXaV4= From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> To: linux-mmc@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Maxim Levitsky <maximlevitsky@gmail.com>, Alex Dubov <oakad@yahoo.com>, Ulf Hansson <ulf.hansson@linaro.org>, "Rafael J. Wysocki" <rafael@kernel.org>, Hans de Goede <hdegoede@redhat.com>, Kay Sievers <kay.sievers@vrfy.org>, stable <stable@kernel.org>, Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr> Subject: [RESEND PATCH] memstick: fix memory leak if card device is never registered Date: Sat, 1 Apr 2023 22:03:27 +0200 Message-Id: <20230401200327.16800-1-gregkh@linuxfoundation.org> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2048; i=gregkh@linuxfoundation.org; h=from:subject; bh=/Fygv1Pu1n1/p5GSibbFK63D+X3dR2C/FWH95CEqEGk=; b=owGbwMvMwCRo6H6F97bub03G02pJDCkafbynebelm4efe19op9yiYBfdNHm3o+EhURXWBEOXnkMb QvZ1xLIwCDIxyIopsnzZxnN0f8UhRS9D29Mwc1iZQIYwcHEKwERSNBgWrPv475Tkh9ZEnfD7vXvUv0 +s8jl3lWF+9ocTKy5nznTIOjHbcOHaq+KuljmvAA== X-Developer-Key: i=gregkh@linuxfoundation.org; a=openpgp; fpr=F4B60CC5BF78C2214A313DCB3147D40DDB2DFB29 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS autolearn=unavailable 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: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762005806379126611?= X-GMAIL-MSGID: =?utf-8?q?1762005806379126611?= |
Series |
[RESEND] memstick: fix memory leak if card device is never registered
|
|
Commit Message
Greg KH
April 1, 2023, 8:03 p.m. UTC
When calling dev_set_name() memory is allocated for the name for the
struct device. Once that structure device is registered, or attempted
to be registerd, with the driver core, the driver core will handle
cleaning up that memory when the device is removed from the system.
Unfortunatly for the memstick code, there is an error path that causes
the struct device to never be registered, and so the memory allocated in
dev_set_name will be leaked. Fix that leak by manually freeing it right
before the memory for the device is freed.
Cc: Maxim Levitsky <maximlevitsky@gmail.com>
Cc: Alex Dubov <oakad@yahoo.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: linux-mmc@vger.kernel.org
Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(),
Cc: stable <stable@kernel.org>
Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
---
RESEND as the first version had a corrupted message id and never made it
to the mailing lists or lore.kernel.org
drivers/memstick/core/memstick.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Comments
On 1.4.2023. 22:03, Greg Kroah-Hartman wrote: > When calling dev_set_name() memory is allocated for the name for the > struct device. Once that structure device is registered, or attempted > to be registerd, with the driver core, the driver core will handle > cleaning up that memory when the device is removed from the system. > > Unfortunatly for the memstick code, there is an error path that causes > the struct device to never be registered, and so the memory allocated in > dev_set_name will be leaked. Fix that leak by manually freeing it right > before the memory for the device is freed. > > Cc: Maxim Levitsky <maximlevitsky@gmail.com> > Cc: Alex Dubov <oakad@yahoo.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Kay Sievers <kay.sievers@vrfy.org> > Cc: linux-mmc@vger.kernel.org > Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(), > Cc: stable <stable@kernel.org> > Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr> > --- > RESEND as the first version had a corrupted message id and never made it > to the mailing lists or lore.kernel.org > > drivers/memstick/core/memstick.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c > index bf7667845459..bbfaf6536903 100644 > --- a/drivers/memstick/core/memstick.c > +++ b/drivers/memstick/core/memstick.c > @@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host) > return card; > err_out: > host->card = old_card; > + kfree_const(card->dev.kobj.name); > kfree(card); > return NULL; > } > @@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work) > put_device(&card->dev); > host->card = NULL; > } > - } else > + } else { > + kfree_const(card->dev.kobj.name); > kfree(card); > + } > } > > out_power_off: FYI, the applied patch tested OK, no memstick leaks: [root@pc-mtodorov kernel]# uname -rms Linux 6.3.0-rc5-mt-20230401-00007-g268a637be362 x86_64 [root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak [root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak What was applied is: mtodorov@domac:~/linux/kernel/linux_torvalds$ git diff master..origin/master | head -24 diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index bbfaf6536903..bf7667845459 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -410,7 +410,6 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host) return card; err_out: host->card = old_card; - kfree_const(card->dev.kobj.name); kfree(card); return NULL; } @@ -469,10 +468,8 @@ static void memstick_check(struct work_struct *work) put_device(&card->dev); host->card = NULL; } - } else { - kfree_const(card->dev.kobj.name); + } else kfree(card); - } } out_power_off: APPENDIX However, please note that the patch SHA-1's truncated to 12 chars might not be the same on the other systems, so the build ID says nothing about which mainline kernel had the patches been applied against. So `uname -rms` is telling pretty nothing about which kernel I'm running, except that it helps me distinguish between the builds. Nothing to prove that: - I am not testing the wrong kernel and - that the right patches have been applied. Changing CONFIG_LOCALVERSION causes copious recompilations, even with ccache it takes about 4x the time needed to compile CONFIG_LOCALVERSION_AUTO=y. Do I make any sense with this? I am adding Cc: to Mr. Bagas, for we spoke already about kernel versioning in the case of manually applied patches. Regards, Mirsad
On Sat, 1 Apr 2023 at 22:03, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > When calling dev_set_name() memory is allocated for the name for the > struct device. Once that structure device is registered, or attempted > to be registerd, with the driver core, the driver core will handle > cleaning up that memory when the device is removed from the system. > > Unfortunatly for the memstick code, there is an error path that causes > the struct device to never be registered, and so the memory allocated in > dev_set_name will be leaked. Fix that leak by manually freeing it right > before the memory for the device is freed. > > Cc: Maxim Levitsky <maximlevitsky@gmail.com> > Cc: Alex Dubov <oakad@yahoo.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Kay Sievers <kay.sievers@vrfy.org> > Cc: linux-mmc@vger.kernel.org > Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(), > Cc: stable <stable@kernel.org> > Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr> Applied for fixes and by adding Mirsad's sob tag (according to the other thread [1]), thanks! Kind regards Uffe [1] https://lore.kernel.org/lkml/c059f486-98a6-aecd-c135-c033612e6b4f@alu.unizg.hr/ > --- > RESEND as the first version had a corrupted message id and never made it > to the mailing lists or lore.kernel.org > > drivers/memstick/core/memstick.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c > index bf7667845459..bbfaf6536903 100644 > --- a/drivers/memstick/core/memstick.c > +++ b/drivers/memstick/core/memstick.c > @@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host) > return card; > err_out: > host->card = old_card; > + kfree_const(card->dev.kobj.name); > kfree(card); > return NULL; > } > @@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work) > put_device(&card->dev); > host->card = NULL; > } > - } else > + } else { > + kfree_const(card->dev.kobj.name); > kfree(card); > + } > } > > out_power_off: > -- > 2.40.0 >
On Tue, Apr 04, 2023 at 01:54:03PM +0200, Ulf Hansson wrote: > On Sat, 1 Apr 2023 at 22:03, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > When calling dev_set_name() memory is allocated for the name for the > > struct device. Once that structure device is registered, or attempted > > to be registerd, with the driver core, the driver core will handle > > cleaning up that memory when the device is removed from the system. > > > > Unfortunatly for the memstick code, there is an error path that causes > > the struct device to never be registered, and so the memory allocated in > > dev_set_name will be leaked. Fix that leak by manually freeing it right > > before the memory for the device is freed. > > > > Cc: Maxim Levitsky <maximlevitsky@gmail.com> > > Cc: Alex Dubov <oakad@yahoo.com> > > Cc: Ulf Hansson <ulf.hansson@linaro.org> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > Cc: Hans de Goede <hdegoede@redhat.com> > > Cc: Kay Sievers <kay.sievers@vrfy.org> > > Cc: linux-mmc@vger.kernel.org > > Fixes: 0252c3b4f018 ("memstick: struct device - replace bus_id with dev_name(), > > Cc: stable <stable@kernel.org> > > Co-developed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Co-developed-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr> > > Applied for fixes and by adding Mirsad's sob tag (according to the > other thread [1]), thanks! Wonderful, thanks for picking up that tag, and the patch. greg k-h
diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index bf7667845459..bbfaf6536903 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -410,6 +410,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host) return card; err_out: host->card = old_card; + kfree_const(card->dev.kobj.name); kfree(card); return NULL; } @@ -468,8 +469,10 @@ static void memstick_check(struct work_struct *work) put_device(&card->dev); host->card = NULL; } - } else + } else { + kfree_const(card->dev.kobj.name); kfree(card); + } } out_power_off: