| Message ID | 20221025113957.693723-1-yangyingliang@huawei.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp955688wru;
Tue, 25 Oct 2022 04:45:54 -0700 (PDT)
X-Google-Smtp-Source:
AMsMyM6X+ae9sEpNwrK/+ZyTCI99UYj28DVPVYyQbga/Rl1y5sDGhJZTG4CIPGZ0MS/vb0FTNrsA
X-Received: by 2002:a17:90a:f414:b0:212:cacf:42c3 with SMTP id
ch20-20020a17090af41400b00212cacf42c3mr27010200pjb.198.1666698354399;
Tue, 25 Oct 2022 04:45:54 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1666698354; cv=none;
d=google.com; s=arc-20160816;
b=XouFI45GtvpUV12j6hR9WeoPS21b4+vy0VQIJGvY6xiakgvAZdC4tfnVE6Eqaax60v
PxDp3n2hYMedacwEsK76qZyWNXfP97+Hpi9kXDyDaXtPavgOM+Y/54XRMGWDqKZNNELH
2PiBr2bokXHuUQ7DgTpZOOotfee5OflLjgs57k/WzIhy13jnvRIHgQgSz91Y+zrvLIvw
PqcGgRqqrWoa9PMWJcnjjXyGrdm9PMuyshhYNoyyJCG8w9+fO6g7uy5bgjr0aRiZiD12
CG/03o+CpJ+i6Cdpa1nQk5wsaFMjlwPeXafeZGL9BP/kGgsCh7DWjAMO2C9yRfOj5CKA
ClNg==
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;
bh=/V6O4TCmnqII10wGdIZVXkV8xBSUzGD7kLd5skW/Vcs=;
b=n1z/BENkWTKFT24Ebu42H2xsBydzmZS4O9uxwZDzn1WmgeySmLQrlZI2AslUTTEy/V
9FErFlT3NAC40Tk+HfzuEmcPms0zUt0ZHzfykgJW766UeW6jJutGqVznqS02y/KHPOoE
Y+OcnxXOAIGeKolgxw2dRED8FS37Y0ofjkNUNsYOZFy6qd/yqazJhijuYcf7vkdxzqnY
yH9VsvgELzCsgdKZ7avOM0OQVi1hDK005Gnf4SkdfCVrdwq8Ql9WPA+bUsVQ8xGi25co
4xUbbRKyZASMVZw2vkyRn5I5ESSiXYT4EcIOXHDggSXzgSXfgfUzga6Kyvumpaqyq1SQ
iZeg==
ARC-Authentication-Results: i=1; mx.google.com;
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=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
y8-20020a056a00190800b005632b7b5e41si3169759pfi.136.2022.10.25.04.45.40;
Tue, 25 Oct 2022 04:45:54 -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;
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=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S231197AbiJYLlA (ORCPT <rfc822;pwkd43@gmail.com> + 99 others);
Tue, 25 Oct 2022 07:41:00 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36448 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229740AbiJYLk6 (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Tue, 25 Oct 2022 07:40:58 -0400
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8A1A3AEA3A;
Tue, 25 Oct 2022 04:40:54 -0700 (PDT)
Received: from dggpemm500021.china.huawei.com (unknown [172.30.72.55])
by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4MxVJ70BnmzVhpm;
Tue, 25 Oct 2022 19:36:07 +0800 (CST)
Received: from dggpemm500007.china.huawei.com (7.185.36.183) by
dggpemm500021.china.huawei.com (7.185.36.109) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2375.31; Tue, 25 Oct 2022 19:40:52 +0800
Received: from huawei.com (10.175.103.91) by dggpemm500007.china.huawei.com
(7.185.36.183) with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Tue, 25 Oct
2022 19:40:52 +0800
From: Yang Yingliang <yangyingliang@huawei.com>
To: <linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
CC: <logang@deltatee.com>, <dan.j.williams@intel.com>,
<hans.verkuil@cisco.com>, <alexandre.belloni@free-electrons.com>,
<gregkh@linuxfoundation.org>, <viro@zeniv.linux.org.uk>
Subject: [PATCH v2] chardev: fix error handling in cdev_device_add()
Date: Tue, 25 Oct 2022 19:39:57 +0800
Message-ID: <20221025113957.693723-1-yangyingliang@huawei.com>
X-Mailer: git-send-email 2.25.1
MIME-Version: 1.0
Content-Transfer-Encoding: 7BIT
Content-Type: text/plain; charset=US-ASCII
X-Originating-IP: [10.175.103.91]
X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To
dggpemm500007.china.huawei.com (7.185.36.183)
X-CFilter-Loop: Reflected
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED,
SPF_HELO_NONE,SPF_PASS 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: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1747659893560608131?=
X-GMAIL-MSGID: =?utf-8?q?1747659893560608131?=
|
| Series |
[v2] chardev: fix error handling in cdev_device_add()
|
|
Commit Message
Yang Yingliang
Oct. 25, 2022, 11:39 a.m. UTC
While doing fault injection test, I got the following report:
------------[ cut here ]------------
kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called.
WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0
CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:kobject_put+0x23d/0x4e0
Call Trace:
<TASK>
cdev_device_add+0x15e/0x1b0
__iio_device_register+0x13b4/0x1af0 [industrialio]
__devm_iio_device_register+0x22/0x90 [industrialio]
max517_probe+0x3d8/0x6b4 [max517]
i2c_device_probe+0xa81/0xc00
When device_add() is injected fault and returns error, if dev->devt is not set,
cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt
in error path.
Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
v1 -> v2:
Add information to update commit message.
v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/
---
fs/char_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: > While doing fault injection test, I got the following report: > > ------------[ cut here ]------------ > kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. > WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 > CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > RIP: 0010:kobject_put+0x23d/0x4e0 > Call Trace: > <TASK> > cdev_device_add+0x15e/0x1b0 > __iio_device_register+0x13b4/0x1af0 [industrialio] > __devm_iio_device_register+0x22/0x90 [industrialio] > max517_probe+0x3d8/0x6b4 [max517] > i2c_device_probe+0xa81/0xc00 > > When device_add() is injected fault and returns error, if dev->devt is not set, > cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt > in error path. Nit, please wrap your changelog text at 72 columns. > > Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > v1 -> v2: > Add information to update commit message. > v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ > --- > fs/char_dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/char_dev.c b/fs/char_dev.c > index ba0ded7842a7..3f667292608c 100644 > --- a/fs/char_dev.c > +++ b/fs/char_dev.c > @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) > } > > rc = device_add(dev); > - if (rc) > + if (rc && dev->devt) No, this is a layering violation and one that you do not know is really going to be true or not. the devt being present, or not, should not be an issue of if the device_add failed or not. This isn't correct, sorry. thanks, greg k-h
Hi, Greg On 2022/10/25 19:50, Greg KH wrote: > On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: >> While doing fault injection test, I got the following report: >> >> ------------[ cut here ]------------ >> kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. >> WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 >> CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >> RIP: 0010:kobject_put+0x23d/0x4e0 >> Call Trace: >> <TASK> >> cdev_device_add+0x15e/0x1b0 >> __iio_device_register+0x13b4/0x1af0 [industrialio] >> __devm_iio_device_register+0x22/0x90 [industrialio] >> max517_probe+0x3d8/0x6b4 [max517] >> i2c_device_probe+0xa81/0xc00 >> >> When device_add() is injected fault and returns error, if dev->devt is not set, >> cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt >> in error path. > Nit, please wrap your changelog text at 72 columns. > >> Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> v1 -> v2: >> Add information to update commit message. >> v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ >> --- >> fs/char_dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/char_dev.c b/fs/char_dev.c >> index ba0ded7842a7..3f667292608c 100644 >> --- a/fs/char_dev.c >> +++ b/fs/char_dev.c >> @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) >> } >> >> rc = device_add(dev); >> - if (rc) >> + if (rc && dev->devt) > No, this is a layering violation and one that you do not know is really > going to be true or not. the devt being present, or not, should not be > an issue of if the device_add failed or not. This isn't correct, sorry. Do you mean it's not a bug or the warn can be ignored or it's bug in driver ? I see devt is checked before calling cdev_del() in cdev_device_del(). Thanks, Yang > > thanks, > > greg k-h > .
On Tue, Oct 25, 2022 at 09:20:12PM +0800, Yang Yingliang wrote: > Hi, Greg > > On 2022/10/25 19:50, Greg KH wrote: > > On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: > > > While doing fault injection test, I got the following report: > > > > > > ------------[ cut here ]------------ > > > kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. > > > WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 > > > CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > > > RIP: 0010:kobject_put+0x23d/0x4e0 > > > Call Trace: > > > <TASK> > > > cdev_device_add+0x15e/0x1b0 > > > __iio_device_register+0x13b4/0x1af0 [industrialio] > > > __devm_iio_device_register+0x22/0x90 [industrialio] > > > max517_probe+0x3d8/0x6b4 [max517] > > > i2c_device_probe+0xa81/0xc00 > > > > > > When device_add() is injected fault and returns error, if dev->devt is not set, > > > cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt > > > in error path. > > Nit, please wrap your changelog text at 72 columns. > > > > > Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > > --- > > > v1 -> v2: > > > Add information to update commit message. > > > v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ > > > --- > > > fs/char_dev.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/fs/char_dev.c b/fs/char_dev.c > > > index ba0ded7842a7..3f667292608c 100644 > > > --- a/fs/char_dev.c > > > +++ b/fs/char_dev.c > > > @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) > > > } > > > rc = device_add(dev); > > > - if (rc) > > > + if (rc && dev->devt) > > No, this is a layering violation and one that you do not know is really > > going to be true or not. the devt being present, or not, should not be > > an issue of if the device_add failed or not. This isn't correct, sorry. > Do you mean it's not a bug or the warn can be ignored or it's bug in driver > ? > I see devt is checked before calling cdev_del() in cdev_device_del(). Ah! The core doesn't set devt, the caller has that set. That makes more sense now, sorry for the confusion on my side. Yes, this looks correct, the diff didn't have the full context and I was confused. I'll go queue this up, very nice work. greg k-h
Hi Greg, On 2022/10/25 21:37, Greg KH wrote: > On Tue, Oct 25, 2022 at 09:20:12PM +0800, Yang Yingliang wrote: >> Hi, Greg >> >> On 2022/10/25 19:50, Greg KH wrote: >>> On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: >>>> While doing fault injection test, I got the following report: >>>> >>>> ------------[ cut here ]------------ >>>> kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. >>>> WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 >>>> CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 >>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >>>> RIP: 0010:kobject_put+0x23d/0x4e0 >>>> Call Trace: >>>> <TASK> >>>> cdev_device_add+0x15e/0x1b0 >>>> __iio_device_register+0x13b4/0x1af0 [industrialio] >>>> __devm_iio_device_register+0x22/0x90 [industrialio] >>>> max517_probe+0x3d8/0x6b4 [max517] >>>> i2c_device_probe+0xa81/0xc00 >>>> >>>> When device_add() is injected fault and returns error, if dev->devt is not set, >>>> cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt >>>> in error path. >>> Nit, please wrap your changelog text at 72 columns. >>> >>>> Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") >>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >>>> --- >>>> v1 -> v2: >>>> Add information to update commit message. >>>> v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ >>>> --- >>>> fs/char_dev.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/char_dev.c b/fs/char_dev.c >>>> index ba0ded7842a7..3f667292608c 100644 >>>> --- a/fs/char_dev.c >>>> +++ b/fs/char_dev.c >>>> @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) >>>> } >>>> rc = device_add(dev); >>>> - if (rc) >>>> + if (rc && dev->devt) >>> No, this is a layering violation and one that you do not know is really >>> going to be true or not. the devt being present, or not, should not be >>> an issue of if the device_add failed or not. This isn't correct, sorry. >> Do you mean it's not a bug or the warn can be ignored or it's bug in driver >> ? >> I see devt is checked before calling cdev_del() in cdev_device_del(). > Ah! The core doesn't set devt, the caller has that set. That makes > more sense now, sorry for the confusion on my side. > > Yes, this looks correct, the diff didn't have the full context and I was > confused. > > I'll go queue this up, very nice work. > > greg k-h I didn't find this patch in your trees, does it been merged? Thanks, Yang > .
On Thu, Dec 01, 2022 at 08:06:44PM +0800, Yang Yingliang wrote: > Hi Greg, > > On 2022/10/25 21:37, Greg KH wrote: > > On Tue, Oct 25, 2022 at 09:20:12PM +0800, Yang Yingliang wrote: > > > Hi, Greg > > > > > > On 2022/10/25 19:50, Greg KH wrote: > > > > On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: > > > > > While doing fault injection test, I got the following report: > > > > > > > > > > ------------[ cut here ]------------ > > > > > kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. > > > > > WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 > > > > > CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 > > > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > > > > > RIP: 0010:kobject_put+0x23d/0x4e0 > > > > > Call Trace: > > > > > <TASK> > > > > > cdev_device_add+0x15e/0x1b0 > > > > > __iio_device_register+0x13b4/0x1af0 [industrialio] > > > > > __devm_iio_device_register+0x22/0x90 [industrialio] > > > > > max517_probe+0x3d8/0x6b4 [max517] > > > > > i2c_device_probe+0xa81/0xc00 > > > > > > > > > > When device_add() is injected fault and returns error, if dev->devt is not set, > > > > > cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt > > > > > in error path. > > > > Nit, please wrap your changelog text at 72 columns. > > > > > > > > > Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") > > > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > > > > > --- > > > > > v1 -> v2: > > > > > Add information to update commit message. > > > > > v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ > > > > > --- > > > > > fs/char_dev.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/fs/char_dev.c b/fs/char_dev.c > > > > > index ba0ded7842a7..3f667292608c 100644 > > > > > --- a/fs/char_dev.c > > > > > +++ b/fs/char_dev.c > > > > > @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) > > > > > } > > > > > rc = device_add(dev); > > > > > - if (rc) > > > > > + if (rc && dev->devt) > > > > No, this is a layering violation and one that you do not know is really > > > > going to be true or not. the devt being present, or not, should not be > > > > an issue of if the device_add failed or not. This isn't correct, sorry. > > > Do you mean it's not a bug or the warn can be ignored or it's bug in driver > > > ? > > > I see devt is checked before calling cdev_del() in cdev_device_del(). > > Ah! The core doesn't set devt, the caller has that set. That makes > > more sense now, sorry for the confusion on my side. > > > > Yes, this looks correct, the diff didn't have the full context and I was > > confused. > > > > I'll go queue this up, very nice work. > > > > greg k-h > I didn't find this patch in your trees, does it been merged? Hm, is this: 4634c973096a ("chardev: Fix potential memory leak when cdev_add() failed") or is this a different patch? If different, it's not in my review queue anymore, sorry, can you resend it? thanks, greg k-h
On 2022/12/1 20:28, Greg KH wrote: > On Thu, Dec 01, 2022 at 08:06:44PM +0800, Yang Yingliang wrote: >> Hi Greg, >> >> On 2022/10/25 21:37, Greg KH wrote: >>> On Tue, Oct 25, 2022 at 09:20:12PM +0800, Yang Yingliang wrote: >>>> Hi, Greg >>>> >>>> On 2022/10/25 19:50, Greg KH wrote: >>>>> On Tue, Oct 25, 2022 at 07:39:57PM +0800, Yang Yingliang wrote: >>>>>> While doing fault injection test, I got the following report: >>>>>> >>>>>> ------------[ cut here ]------------ >>>>>> kobject: '(null)' (0000000039956980): is not initialized, yet kobject_put() is being called. >>>>>> WARNING: CPU: 3 PID: 6306 at kobject_put+0x23d/0x4e0 >>>>>> CPU: 3 PID: 6306 Comm: 283 Tainted: G W 6.1.0-rc2-00005-g307c1086d7c9 #1253 >>>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >>>>>> RIP: 0010:kobject_put+0x23d/0x4e0 >>>>>> Call Trace: >>>>>> <TASK> >>>>>> cdev_device_add+0x15e/0x1b0 >>>>>> __iio_device_register+0x13b4/0x1af0 [industrialio] >>>>>> __devm_iio_device_register+0x22/0x90 [industrialio] >>>>>> max517_probe+0x3d8/0x6b4 [max517] >>>>>> i2c_device_probe+0xa81/0xc00 >>>>>> >>>>>> When device_add() is injected fault and returns error, if dev->devt is not set, >>>>>> cdev_add() is not called, cdev_del() is not needed. Fix this by checking dev->devt >>>>>> in error path. >>>>> Nit, please wrap your changelog text at 72 columns. >>>>> >>>>>> Fixes: 233ed09d7fda ("chardev: add helper function to register char devs with a struct device") >>>>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >>>>>> --- >>>>>> v1 -> v2: >>>>>> Add information to update commit message. >>>>>> v1 link: https://lore.kernel.org/lkml/1959fa74-b06c-b8bc-d14f-b71e5c4290ee@huawei.com/T/ >>>>>> --- >>>>>> fs/char_dev.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/fs/char_dev.c b/fs/char_dev.c >>>>>> index ba0ded7842a7..3f667292608c 100644 >>>>>> --- a/fs/char_dev.c >>>>>> +++ b/fs/char_dev.c >>>>>> @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) >>>>>> } >>>>>> rc = device_add(dev); >>>>>> - if (rc) >>>>>> + if (rc && dev->devt) >>>>> No, this is a layering violation and one that you do not know is really >>>>> going to be true or not. the devt being present, or not, should not be >>>>> an issue of if the device_add failed or not. This isn't correct, sorry. >>>> Do you mean it's not a bug or the warn can be ignored or it's bug in driver >>>> ? >>>> I see devt is checked before calling cdev_del() in cdev_device_del(). >>> Ah! The core doesn't set devt, the caller has that set. That makes >>> more sense now, sorry for the confusion on my side. >>> >>> Yes, this looks correct, the diff didn't have the full context and I was >>> confused. >>> >>> I'll go queue this up, very nice work. >>> >>> greg k-h >> I didn't find this patch in your trees, does it been merged? > Hm, is this: > 4634c973096a ("chardev: Fix potential memory leak when cdev_add() failed") > or is this a different patch? If different, it's not in my review queue > anymore, sorry, can you resend it? It's a different patch, I can resend it. Thanks, Yang > > thanks, > > greg k-h > .
diff --git a/fs/char_dev.c b/fs/char_dev.c index ba0ded7842a7..3f667292608c 100644 --- a/fs/char_dev.c +++ b/fs/char_dev.c @@ -547,7 +547,7 @@ int cdev_device_add(struct cdev *cdev, struct device *dev) } rc = device_add(dev); - if (rc) + if (rc && dev->devt) cdev_del(cdev); return rc;