Message ID | 20221118032827.3725190-1-ruanjinjie@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp760621wrr; Thu, 17 Nov 2022 19:47:48 -0800 (PST) X-Google-Smtp-Source: AA0mqf7vMfvjY9xMdtDM6NkxPz06Z8xDrRAJi8g7KXqNsgyZ/i3D0zqAAFCkpj8T51ys1LSLFxcB X-Received: by 2002:a17:906:79d3:b0:78d:b6f7:53c0 with SMTP id m19-20020a17090679d300b0078db6f753c0mr4571170ejo.527.1668743267987; Thu, 17 Nov 2022 19:47:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668743267; cv=none; d=google.com; s=arc-20160816; b=C3/I3xUkUvZ+uxLZ/bUhZKqQQWdK020hJ+mHwXW+ijeiSLO1POUc2cATar5v7dsQOF LG39x/5I3sgh0dT8MHtRcrwurFltMBaPSH0LCToPU6sHJa99CVsWuDOYgIHqq0s/JP1L by45kz4wQDhguMLT8yHOhHUOv7RqqwWVle0shKb3vT2OVvRmp6xHi9QurhzFBayr6Iz/ ZVm+tQk2kjRaMx3YEyDtugRONA2sp5Yrx30KlvGHeuYyqvByI2heCnAMyD+n8cHO8UsJ Bp+zI1ap7iIqZpNa8D47sQAfaIq5lMr3PlVdRBobyAIa8ukfywLKJqS5LamwxLOUTXsn 1amw== 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=uWxeybHKQpA1Mzf6BwVMJyGwocI56R5F62jv+3qu8iQ=; b=Dg0GxPAL0V/M8hAEkziwZFJS5cqIfcwMbgZf9oeSntE81Gcp542ZQmaLTLu091D640 K0q6sHJW5O0iZdyf4k6uOAiE3VjknbbNZEcn3oQ4718VZeov3Bmg5adkNdNetxXZnOrg ymKoEn2XAEeicHSQvCybVNph2pmivfbjvDgo5EenVfQWoeqWtYezM+mcCS3u2ePbS/oP Ra9Vh5GaL/QSrl50F3NDRVjTFnowtkrq7xzyZuFO/45AUw/wmghvXGRT2js92SpPET3y f4kq7udUgc33AVbiJOFF2DH7qio9VusftAXV+tfAySSfVYUAzk/dMr2lvPH/F+LfhHnI VapQ== 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 ae13-20020a17090725cd00b00775326fb2ebsi1739227ejc.665.2022.11.17.19.47.23; Thu, 17 Nov 2022 19:47:47 -0800 (PST) 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 S240679AbiKRDbo (ORCPT <rfc822;a1648639935@gmail.com> + 99 others); Thu, 17 Nov 2022 22:31:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40582 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234758AbiKRDbm (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 17 Nov 2022 22:31:42 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5D07B5132F; Thu, 17 Nov 2022 19:31:40 -0800 (PST) Received: from kwepemi500008.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4ND2PP6fV7zHvtg; Fri, 18 Nov 2022 11:31:05 +0800 (CST) Received: from huawei.com (10.67.175.83) by kwepemi500008.china.huawei.com (7.221.188.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Fri, 18 Nov 2022 11:31:37 +0800 From: ruanjinjie <ruanjinjie@huawei.com> To: <kwankhede@nvidia.com>, <alex.williamson@redhat.com>, <kraxel@redhat.com>, <cjia@nvidia.com>, <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org> CC: <ruanjinjie@huawei.com> Subject: [PATCH] vfio/mdev: fix possible memory leak in module init funcs Date: Fri, 18 Nov 2022 11:28:27 +0800 Message-ID: <20221118032827.3725190-1-ruanjinjie@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.67.175.83] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemi500008.china.huawei.com (7.221.188.139) 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?1749804141022016290?= X-GMAIL-MSGID: =?utf-8?q?1749804141022016290?= |
Series |
vfio/mdev: fix possible memory leak in module init funcs
|
|
Commit Message
Jinjie Ruan
Nov. 18, 2022, 3:28 a.m. UTC
Inject fault while probing module, if device_register() fails,
but the refcount of kobject is not decreased to 0, the name
allocated in dev_set_name() is leaked. Fix this by calling
put_device(), so that name can be freed in callback function
kobject_cleanup().
unreferenced object 0xffff88807d687008 (size 8):
comm "modprobe", pid 8280, jiffies 4294807686 (age 12.378s)
hex dump (first 8 bytes):
6d 64 70 79 00 6b 6b a5 mdpy.kk.
backtrace:
[<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150
[<ffffffff81731d53>] kstrdup+0x33/0x60
[<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110
[<ffffffff82d91abb>] dev_set_name+0xab/0xe0
[<ffffffffa0260105>] 0xffffffffa0260105
[<ffffffff81001c27>] do_one_initcall+0x87/0x2e0
[<ffffffff813739cb>] do_init_module+0x1ab/0x640
[<ffffffff81379d20>] load_module+0x5d00/0x77f0
[<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0
[<ffffffff83c944a5>] do_syscall_64+0x35/0x80
[<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
unreferenced object 0xffff888101ccbcf8 (size 8):
comm "modprobe", pid 15662, jiffies 4295164481 (age 13.282s)
hex dump (first 8 bytes):
6d 74 74 79 00 6b 6b a5 mtty.kk.
backtrace:
[<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150
[<ffffffff81731d53>] kstrdup+0x33/0x60
[<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110
[<ffffffff82d91abb>] dev_set_name+0xab/0xe0
[<ffffffffa0248134>] 0xffffffffa0248134
[<ffffffff81001c27>] do_one_initcall+0x87/0x2e0
[<ffffffff813739cb>] do_init_module+0x1ab/0x640
[<ffffffff81379d20>] load_module+0x5d00/0x77f0
[<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0
[<ffffffff83c944a5>] do_syscall_64+0x35/0x80
[<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
unreferenced object 0xffff88810177c6c8 (size 8):
comm "modprobe", pid 23657, jiffies 4295314656 (age 13.227s)
hex dump (first 8 bytes):
6d 62 6f 63 68 73 00 a5 mbochs..
backtrace:
[<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150
[<ffffffff81731d53>] kstrdup+0x33/0x60
[<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110
[<ffffffff82d91abb>] dev_set_name+0xab/0xe0
[<ffffffffa0248124>] 0xffffffffa0248124
[<ffffffff81001c27>] do_one_initcall+0x87/0x2e0
[<ffffffff813739cb>] do_init_module+0x1ab/0x640
[<ffffffff81379d20>] load_module+0x5d00/0x77f0
[<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0
[<ffffffff83c944a5>] do_syscall_64+0x35/0x80
[<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
Fixes: d61fc96f47fd ("sample: vfio mdev display - host device")
Fixes: 9d1a546c53b4 ("docs: Sample driver to demonstrate how to use Mediated device framework.")
Fixes: a5e6e6505f38 ("sample: vfio bochs vbe display (host device for bochs-drm)")
Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
---
samples/vfio-mdev/mbochs.c | 4 +++-
samples/vfio-mdev/mdpy.c | 4 +++-
samples/vfio-mdev/mtty.c | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
Comments
Ping. On 2022/11/18 11:28, ruanjinjie wrote: > Inject fault while probing module, if device_register() fails, > but the refcount of kobject is not decreased to 0, the name > allocated in dev_set_name() is leaked. Fix this by calling > put_device(), so that name can be freed in callback function > kobject_cleanup(). > > unreferenced object 0xffff88807d687008 (size 8): > comm "modprobe", pid 8280, jiffies 4294807686 (age 12.378s) > hex dump (first 8 bytes): > 6d 64 70 79 00 6b 6b a5 mdpy.kk. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0260105>] 0xffffffffa0260105 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > unreferenced object 0xffff888101ccbcf8 (size 8): > comm "modprobe", pid 15662, jiffies 4295164481 (age 13.282s) > hex dump (first 8 bytes): > 6d 74 74 79 00 6b 6b a5 mtty.kk. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0248134>] 0xffffffffa0248134 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > unreferenced object 0xffff88810177c6c8 (size 8): > comm "modprobe", pid 23657, jiffies 4295314656 (age 13.227s) > hex dump (first 8 bytes): > 6d 62 6f 63 68 73 00 a5 mbochs.. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0248124>] 0xffffffffa0248124 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > Fixes: d61fc96f47fd ("sample: vfio mdev display - host device") > Fixes: 9d1a546c53b4 ("docs: Sample driver to demonstrate how to use Mediated device framework.") > Fixes: a5e6e6505f38 ("sample: vfio bochs vbe display (host device for bochs-drm)") > Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> > --- > samples/vfio-mdev/mbochs.c | 4 +++- > samples/vfio-mdev/mdpy.c | 4 +++- > samples/vfio-mdev/mtty.c | 4 +++- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c > index 117a8d799f71..1c47672be815 100644 > --- a/samples/vfio-mdev/mbochs.c > +++ b/samples/vfio-mdev/mbochs.c > @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) > dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); > > ret = device_register(&mbochs_dev); > - if (ret) > + if (ret) { > + put_device(&mbochs_dev); > goto err_class; > + } > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, &mbochs_driver, > mbochs_mdev_types, > diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c > index 946e8cfde6fd..bfb93eaf535b 100644 > --- a/samples/vfio-mdev/mdpy.c > +++ b/samples/vfio-mdev/mdpy.c > @@ -717,8 +717,10 @@ static int __init mdpy_dev_init(void) > dev_set_name(&mdpy_dev, "%s", MDPY_NAME); > > ret = device_register(&mdpy_dev); > - if (ret) > + if (ret) { > + put_device(&mdpy_dev); > goto err_class; > + } > > ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, &mdpy_driver, > mdpy_mdev_types, > diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c > index e72085fc1376..dddb0619846c 100644 > --- a/samples/vfio-mdev/mtty.c > +++ b/samples/vfio-mdev/mtty.c > @@ -1330,8 +1330,10 @@ static int __init mtty_dev_init(void) > dev_set_name(&mtty_dev.dev, "%s", MTTY_NAME); > > ret = device_register(&mtty_dev.dev); > - if (ret) > + if (ret) { > + put_device(&mtty_dev.dev); > goto err_class; > + } > > ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, > &mtty_driver, mtty_mdev_types,
[Cc +vfio-ap, vfio-ccw] On Fri, 18 Nov 2022 11:28:27 +0800 ruanjinjie <ruanjinjie@huawei.com> wrote: > Inject fault while probing module, if device_register() fails, > but the refcount of kobject is not decreased to 0, the name > allocated in dev_set_name() is leaked. Fix this by calling > put_device(), so that name can be freed in callback function > kobject_cleanup(). > > unreferenced object 0xffff88807d687008 (size 8): > comm "modprobe", pid 8280, jiffies 4294807686 (age 12.378s) > hex dump (first 8 bytes): > 6d 64 70 79 00 6b 6b a5 mdpy.kk. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0260105>] 0xffffffffa0260105 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > unreferenced object 0xffff888101ccbcf8 (size 8): > comm "modprobe", pid 15662, jiffies 4295164481 (age 13.282s) > hex dump (first 8 bytes): > 6d 74 74 79 00 6b 6b a5 mtty.kk. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0248134>] 0xffffffffa0248134 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > unreferenced object 0xffff88810177c6c8 (size 8): > comm "modprobe", pid 23657, jiffies 4295314656 (age 13.227s) > hex dump (first 8 bytes): > 6d 62 6f 63 68 73 00 a5 mbochs.. > backtrace: > [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 > [<ffffffff81731d53>] kstrdup+0x33/0x60 > [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 > [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 > [<ffffffffa0248124>] 0xffffffffa0248124 > [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 > [<ffffffff813739cb>] do_init_module+0x1ab/0x640 > [<ffffffff81379d20>] load_module+0x5d00/0x77f0 > [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 > [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 > [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > Fixes: d61fc96f47fd ("sample: vfio mdev display - host device") > Fixes: 9d1a546c53b4 ("docs: Sample driver to demonstrate how to use Mediated device framework.") > Fixes: a5e6e6505f38 ("sample: vfio bochs vbe display (host device for bochs-drm)") > Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> > --- > samples/vfio-mdev/mbochs.c | 4 +++- > samples/vfio-mdev/mdpy.c | 4 +++- > samples/vfio-mdev/mtty.c | 4 +++- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c > index 117a8d799f71..1c47672be815 100644 > --- a/samples/vfio-mdev/mbochs.c > +++ b/samples/vfio-mdev/mbochs.c > @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) > dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); > > ret = device_register(&mbochs_dev); > - if (ret) > + if (ret) { > + put_device(&mbochs_dev); > goto err_class; > + } > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, &mbochs_driver, > mbochs_mdev_types, vfio-ap has a similar unwind as the sample drivers, but actually makes an attempt to catch this ex: ... ret = device_register(&matrix_dev->device); if (ret) goto matrix_reg_err; ret = driver_register(&matrix_driver); if (ret) goto matrix_drv_err; return 0; matrix_drv_err: device_unregister(&matrix_dev->device); matrix_reg_err: put_device(&matrix_dev->device); ... So of the vfio drivers calling device_register(), vfio-ap is the only one that does a put_device() if device_register() fails, but it also seems sketchy to call both device_unregister() and put_device() in the case that we exit via matrix_drv_err. I wonder if all of these shouldn't adopt a flow like: ret = device_register(&dev); if (ret) goto err1; .... return 0; err2: device_del(&dev); err1: put_device(&dev); Thanks, Alex > diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c > index 946e8cfde6fd..bfb93eaf535b 100644 > --- a/samples/vfio-mdev/mdpy.c > +++ b/samples/vfio-mdev/mdpy.c > @@ -717,8 +717,10 @@ static int __init mdpy_dev_init(void) > dev_set_name(&mdpy_dev, "%s", MDPY_NAME); > > ret = device_register(&mdpy_dev); > - if (ret) > + if (ret) { > + put_device(&mdpy_dev); > goto err_class; > + } > > ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, &mdpy_driver, > mdpy_mdev_types, > diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c > index e72085fc1376..dddb0619846c 100644 > --- a/samples/vfio-mdev/mtty.c > +++ b/samples/vfio-mdev/mtty.c > @@ -1330,8 +1330,10 @@ static int __init mtty_dev_init(void) > dev_set_name(&mtty_dev.dev, "%s", MTTY_NAME); > > ret = device_register(&mtty_dev.dev); > - if (ret) > + if (ret) { > + put_device(&mtty_dev.dev); > goto err_class; > + } > > ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, > &mtty_driver, mtty_mdev_types,
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Thursday, December 1, 2022 7:06 AM > > [Cc +vfio-ap, vfio-ccw] > > On Fri, 18 Nov 2022 11:28:27 +0800 > ruanjinjie <ruanjinjie@huawei.com> wrote: > > > Inject fault while probing module, if device_register() fails, > > but the refcount of kobject is not decreased to 0, the name > > allocated in dev_set_name() is leaked. Fix this by calling > > put_device(), so that name can be freed in callback function > > kobject_cleanup(). It's not just about the name. The problem of kboject not being released is a bigger one. put_device() is always required no matter device_register() succeeds or not: * NOTE: _Never_ directly free @dev after calling this function, even * if it returned an error! Always use put_device() to give up the * reference initialized in this function instead. */ int device_register(struct device *dev) > > @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) > > dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); > > > > ret = device_register(&mbochs_dev); > > - if (ret) > > + if (ret) { > > + put_device(&mbochs_dev); > > goto err_class; > > + } > > > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, > &mbochs_driver, > > mbochs_mdev_types, > > > vfio-ap has a similar unwind as the sample drivers, but actually makes > an attempt to catch this ex: > > ... > ret = device_register(&matrix_dev->device); > if (ret) > goto matrix_reg_err; > > ret = driver_register(&matrix_driver); > if (ret) > goto matrix_drv_err; > > return 0; > > matrix_drv_err: > device_unregister(&matrix_dev->device); > matrix_reg_err: > put_device(&matrix_dev->device); > ... > > So of the vfio drivers calling device_register(), vfio-ap is the only > one that does a put_device() if device_register() fails, but it also > seems sketchy to call both device_unregister() and put_device() in the > case that we exit via matrix_drv_err. > > I wonder if all of these shouldn't adopt a flow like: > > ret = device_register(&dev); > if (ret) > goto err1; > > .... > > return 0; > > err2: > device_del(&dev); > err1: > put_device(&dev); > It's kind of a mixed model. With above unwind it's clearer to use device_initialize() and device_add() instead. Otherwise what this patch does looks better IMHO: ret = device_register(&dev); if (ret) { put_device(&dev); goto err1; } ... return 0; err2: device_unregister(&dev); err1: earlier_unwind();
On 2022/12/1 7:06, Alex Williamson wrote: > [Cc +vfio-ap, vfio-ccw] > > On Fri, 18 Nov 2022 11:28:27 +0800 > ruanjinjie <ruanjinjie@huawei.com> wrote: > >> Inject fault while probing module, if device_register() fails, >> but the refcount of kobject is not decreased to 0, the name >> allocated in dev_set_name() is leaked. Fix this by calling >> put_device(), so that name can be freed in callback function >> kobject_cleanup(). >> >> unreferenced object 0xffff88807d687008 (size 8): >> comm "modprobe", pid 8280, jiffies 4294807686 (age 12.378s) >> hex dump (first 8 bytes): >> 6d 64 70 79 00 6b 6b a5 mdpy.kk. >> backtrace: >> [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 >> [<ffffffff81731d53>] kstrdup+0x33/0x60 >> [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 >> [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 >> [<ffffffffa0260105>] 0xffffffffa0260105 >> [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 >> [<ffffffff813739cb>] do_init_module+0x1ab/0x640 >> [<ffffffff81379d20>] load_module+0x5d00/0x77f0 >> [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 >> [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 >> [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 >> >> unreferenced object 0xffff888101ccbcf8 (size 8): >> comm "modprobe", pid 15662, jiffies 4295164481 (age 13.282s) >> hex dump (first 8 bytes): >> 6d 74 74 79 00 6b 6b a5 mtty.kk. >> backtrace: >> [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 >> [<ffffffff81731d53>] kstrdup+0x33/0x60 >> [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 >> [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 >> [<ffffffffa0248134>] 0xffffffffa0248134 >> [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 >> [<ffffffff813739cb>] do_init_module+0x1ab/0x640 >> [<ffffffff81379d20>] load_module+0x5d00/0x77f0 >> [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 >> [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 >> [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 >> >> unreferenced object 0xffff88810177c6c8 (size 8): >> comm "modprobe", pid 23657, jiffies 4295314656 (age 13.227s) >> hex dump (first 8 bytes): >> 6d 62 6f 63 68 73 00 a5 mbochs.. >> backtrace: >> [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150 >> [<ffffffff81731d53>] kstrdup+0x33/0x60 >> [<ffffffff83aa1421>] kobject_set_name_vargs+0x41/0x110 >> [<ffffffff82d91abb>] dev_set_name+0xab/0xe0 >> [<ffffffffa0248124>] 0xffffffffa0248124 >> [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0 >> [<ffffffff813739cb>] do_init_module+0x1ab/0x640 >> [<ffffffff81379d20>] load_module+0x5d00/0x77f0 >> [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0 >> [<ffffffff83c944a5>] do_syscall_64+0x35/0x80 >> [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 >> >> Fixes: d61fc96f47fd ("sample: vfio mdev display - host device") >> Fixes: 9d1a546c53b4 ("docs: Sample driver to demonstrate how to use Mediated device framework.") >> Fixes: a5e6e6505f38 ("sample: vfio bochs vbe display (host device for bochs-drm)") >> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> >> --- >> samples/vfio-mdev/mbochs.c | 4 +++- >> samples/vfio-mdev/mdpy.c | 4 +++- >> samples/vfio-mdev/mtty.c | 4 +++- >> 3 files changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c >> index 117a8d799f71..1c47672be815 100644 >> --- a/samples/vfio-mdev/mbochs.c >> +++ b/samples/vfio-mdev/mbochs.c >> @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) >> dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); >> >> ret = device_register(&mbochs_dev); >> - if (ret) >> + if (ret) { >> + put_device(&mbochs_dev); >> goto err_class; >> + } >> >> ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, &mbochs_driver, >> mbochs_mdev_types, > > > vfio-ap has a similar unwind as the sample drivers, but actually makes > an attempt to catch this ex: I think the reason is vfio-ap driver error path has common unwind to do before device_register, otherwise it can return just after put_device or device_unregister. > > ... > ret = device_register(&matrix_dev->device); > if (ret) > goto matrix_reg_err; > > ret = driver_register(&matrix_driver); > if (ret) > goto matrix_drv_err; > > return 0; > > matrix_drv_err: > device_unregister(&matrix_dev->device); > matrix_reg_err: > put_device(&matrix_dev->device); > ... > > So of the vfio drivers calling device_register(), vfio-ap is the only > one that does a put_device() if device_register() fails, but it also > seems sketchy to call both device_unregister() and put_device() in the > case that we exit via matrix_drv_err. The patch do not change the original error path, just add missing put_device out of the normal error path if device_register fails , so there is no risk of calling both device_unregister() and put_device(). > > I wonder if all of these shouldn't adopt a flow like: > > ret = device_register(&dev); > if (ret) > goto err1; > > .... > > return 0; > > err2: > device_del(&dev); > err1: > put_device(&dev); > > Thanks, > > Alex > >> diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c >> index 946e8cfde6fd..bfb93eaf535b 100644 >> --- a/samples/vfio-mdev/mdpy.c >> +++ b/samples/vfio-mdev/mdpy.c >> @@ -717,8 +717,10 @@ static int __init mdpy_dev_init(void) >> dev_set_name(&mdpy_dev, "%s", MDPY_NAME); >> >> ret = device_register(&mdpy_dev); >> - if (ret) >> + if (ret) { >> + put_device(&mdpy_dev); >> goto err_class; >> + } >> >> ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, &mdpy_driver, >> mdpy_mdev_types, >> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c >> index e72085fc1376..dddb0619846c 100644 >> --- a/samples/vfio-mdev/mtty.c >> +++ b/samples/vfio-mdev/mtty.c >> @@ -1330,8 +1330,10 @@ static int __init mtty_dev_init(void) >> dev_set_name(&mtty_dev.dev, "%s", MTTY_NAME); >> >> ret = device_register(&mtty_dev.dev); >> - if (ret) >> + if (ret) { >> + put_device(&mtty_dev.dev); >> goto err_class; >> + } >> >> ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, >> &mtty_driver, mtty_mdev_types, >
On Thu, 1 Dec 2022 02:00:57 +0000 "Tian, Kevin" <kevin.tian@intel.com> wrote: > > From: Alex Williamson <alex.williamson@redhat.com> > > Sent: Thursday, December 1, 2022 7:06 AM > > > > [Cc +vfio-ap, vfio-ccw] > > > > On Fri, 18 Nov 2022 11:28:27 +0800 > > ruanjinjie <ruanjinjie@huawei.com> wrote: > > > > > Inject fault while probing module, if device_register() fails, > > > but the refcount of kobject is not decreased to 0, the name > > > allocated in dev_set_name() is leaked. Fix this by calling > > > put_device(), so that name can be freed in callback function > > > kobject_cleanup(). > > It's not just about the name. The problem of kboject not being > released is a bigger one. > > put_device() is always required no matter device_register() > succeeds or not: > > * NOTE: _Never_ directly free @dev after calling this function, even > * if it returned an error! Always use put_device() to give up the > * reference initialized in this function instead. > */ > int device_register(struct device *dev) > > > > @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) > > > dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); > > > > > > ret = device_register(&mbochs_dev); > > > - if (ret) > > > + if (ret) { > > > + put_device(&mbochs_dev); > > > goto err_class; > > > + } > > > > > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, > > &mbochs_driver, > > > mbochs_mdev_types, > > > > > > vfio-ap has a similar unwind as the sample drivers, but actually makes > > an attempt to catch this ex: > > > > ... > > ret = device_register(&matrix_dev->device); > > if (ret) > > goto matrix_reg_err; > > > > ret = driver_register(&matrix_driver); > > if (ret) > > goto matrix_drv_err; > > > > return 0; > > > > matrix_drv_err: > > device_unregister(&matrix_dev->device); > > matrix_reg_err: > > put_device(&matrix_dev->device); > > ... > > > > So of the vfio drivers calling device_register(), vfio-ap is the only > > one that does a put_device() if device_register() fails, but it also > > seems sketchy to call both device_unregister() and put_device() in the > > case that we exit via matrix_drv_err. > > > > I wonder if all of these shouldn't adopt a flow like: > > > > ret = device_register(&dev); > > if (ret) > > goto err1; > > > > .... > > > > return 0; > > > > err2: > > device_del(&dev); > > err1: > > put_device(&dev); > > > > It's kind of a mixed model. > > With above unwind it's clearer to use device_initialize() and device_add() instead. That would go against the comment for device_register() recommending that device_initialize() and device_add() should only be called separately if we have a clearly defined need. I can only imagine a patch bot would quickly come along to rectify the situation if we simply open code device_register() for aesthetics. I don't see that splitting device_unregister() for the purpose of having a common unwind path necessitates any changes relative to device_register(). > Otherwise what this patch does looks better IMHO: > > ret = device_register(&dev); > if (ret) { > put_device(&dev); > goto err1; > } > > ... > > return 0; > > err2: > device_unregister(&dev); > err1: > earlier_unwind(); > This is essentially what was originally proposed. It could also be called a "mixed model", implementing part of the unwind in the error branch before jumping to the common unwind. As demonstrated below, every current vfio driver calling device_register() follows a similar goto unwind stack as found in the sample drivers, which makes it trivially easy to split the device_unregister() call and add a goto target in between. Either way, they're equivalent and I'll take whichever version addresses all the vfio related use cases and gets acks from their maintainers. Thanks, Alex diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index c2a65808605a..54aba7cceb33 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -199,8 +199,9 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) return 0; out_unreg: - device_unregister(&parent->dev); + device_del(&parent->dev); out_free: + put_device(&parent->dev); dev_set_drvdata(&sch->dev, NULL); return ret; } diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c index f43cfeabd2cc..997b524bdd2b 100644 --- a/drivers/s390/crypto/vfio_ap_drv.c +++ b/drivers/s390/crypto/vfio_ap_drv.c @@ -122,7 +122,7 @@ static int vfio_ap_matrix_dev_create(void) return 0; matrix_drv_err: - device_unregister(&matrix_dev->device); + device_del(&matrix_dev->device); matrix_reg_err: put_device(&matrix_dev->device); matrix_alloc_err: diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index 8b5a3a778a25..e54eb752e1ba 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -1430,7 +1430,7 @@ static int __init mbochs_dev_init(void) ret = device_register(&mbochs_dev); if (ret) - goto err_class; + goto err_put; ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, &mbochs_driver, mbochs_mdev_types, @@ -1441,8 +1441,9 @@ static int __init mbochs_dev_init(void) return 0; err_device: - device_unregister(&mbochs_dev); -err_class: + device_del(&mbochs_dev); +err_put: + put_device(&mbochs_dev); class_destroy(mbochs_class); err_driver: mdev_unregister_driver(&mbochs_driver); diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index 721fb06c6413..e8400fdab71d 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -717,7 +717,7 @@ static int __init mdpy_dev_init(void) ret = device_register(&mdpy_dev); if (ret) - goto err_class; + goto err_put; ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, &mdpy_driver, mdpy_mdev_types, @@ -728,8 +728,9 @@ static int __init mdpy_dev_init(void) return 0; err_device: - device_unregister(&mdpy_dev); -err_class: + device_del(&mdpy_dev); +err_put: + put_device(&mdpy_dev); class_destroy(mdpy_class); err_driver: mdev_unregister_driver(&mdpy_driver); diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index 3c2a421b9b69..e887de672c52 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1330,7 +1330,7 @@ static int __init mtty_dev_init(void) ret = device_register(&mtty_dev.dev); if (ret) - goto err_class; + goto err_put; ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, &mtty_driver, mtty_mdev_types, @@ -1340,8 +1340,9 @@ static int __init mtty_dev_init(void) return 0; err_device: - device_unregister(&mtty_dev.dev); -err_class: + device_del(&mtty_dev.dev); +err_put: + put_device(&mtty_dev.dev); class_destroy(mtty_dev.vd_class); err_driver: mdev_unregister_driver(&mtty_driver);
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Friday, December 2, 2022 5:10 AM > > > Otherwise what this patch does looks better IMHO: > > > > ret = device_register(&dev); > > if (ret) { > > put_device(&dev); > > goto err1; > > } > > > > ... > > > > return 0; > > > > err2: > > device_unregister(&dev); > > err1: > > earlier_unwind(); > > > > This is essentially what was originally proposed. It could also be > called a "mixed model", implementing part of the unwind in the error > branch before jumping to the common unwind. As demonstrated below, > every current vfio driver calling device_register() follows a similar > goto unwind stack as found in the sample drivers, which makes it > trivially easy to split the device_unregister() call and add a goto > target in between. OK. Actually looking into other users of device_register() I can see both schemes are used, so... > > Either way, they're equivalent and I'll take whichever version > addresses all the vfio related use cases and gets acks from their > maintainers. Thanks, ... agree either way is fine. Since your version fixes all vfio cases, feel free to include: Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > Alex > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c > b/drivers/s390/cio/vfio_ccw_drv.c > index c2a65808605a..54aba7cceb33 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -199,8 +199,9 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) > return 0; > > out_unreg: > - device_unregister(&parent->dev); > + device_del(&parent->dev); > out_free: > + put_device(&parent->dev); > dev_set_drvdata(&sch->dev, NULL); > return ret; > } > diff --git a/drivers/s390/crypto/vfio_ap_drv.c > b/drivers/s390/crypto/vfio_ap_drv.c > index f43cfeabd2cc..997b524bdd2b 100644 > --- a/drivers/s390/crypto/vfio_ap_drv.c > +++ b/drivers/s390/crypto/vfio_ap_drv.c > @@ -122,7 +122,7 @@ static int vfio_ap_matrix_dev_create(void) > return 0; > > matrix_drv_err: > - device_unregister(&matrix_dev->device); > + device_del(&matrix_dev->device); > matrix_reg_err: > put_device(&matrix_dev->device); > matrix_alloc_err: > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c > index 8b5a3a778a25..e54eb752e1ba 100644 > --- a/samples/vfio-mdev/mbochs.c > +++ b/samples/vfio-mdev/mbochs.c > @@ -1430,7 +1430,7 @@ static int __init mbochs_dev_init(void) > > ret = device_register(&mbochs_dev); > if (ret) > - goto err_class; > + goto err_put; > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, > &mbochs_driver, > mbochs_mdev_types, > @@ -1441,8 +1441,9 @@ static int __init mbochs_dev_init(void) > return 0; > > err_device: > - device_unregister(&mbochs_dev); > -err_class: > + device_del(&mbochs_dev); > +err_put: > + put_device(&mbochs_dev); > class_destroy(mbochs_class); > err_driver: > mdev_unregister_driver(&mbochs_driver); > diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c > index 721fb06c6413..e8400fdab71d 100644 > --- a/samples/vfio-mdev/mdpy.c > +++ b/samples/vfio-mdev/mdpy.c > @@ -717,7 +717,7 @@ static int __init mdpy_dev_init(void) > > ret = device_register(&mdpy_dev); > if (ret) > - goto err_class; > + goto err_put; > > ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, > &mdpy_driver, > mdpy_mdev_types, > @@ -728,8 +728,9 @@ static int __init mdpy_dev_init(void) > return 0; > > err_device: > - device_unregister(&mdpy_dev); > -err_class: > + device_del(&mdpy_dev); > +err_put: > + put_device(&mdpy_dev); > class_destroy(mdpy_class); > err_driver: > mdev_unregister_driver(&mdpy_driver); > diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c > index 3c2a421b9b69..e887de672c52 100644 > --- a/samples/vfio-mdev/mtty.c > +++ b/samples/vfio-mdev/mtty.c > @@ -1330,7 +1330,7 @@ static int __init mtty_dev_init(void) > > ret = device_register(&mtty_dev.dev); > if (ret) > - goto err_class; > + goto err_put; > > ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, > &mtty_driver, mtty_mdev_types, > @@ -1340,8 +1340,9 @@ static int __init mtty_dev_init(void) > return 0; > > err_device: > - device_unregister(&mtty_dev.dev); > -err_class: > + device_del(&mtty_dev.dev); > +err_put: > + put_device(&mtty_dev.dev); > class_destroy(mtty_dev.vd_class); > err_driver: > mdev_unregister_driver(&mtty_driver);
On Fri, 2022-12-02 at 05:28 +0000, Tian, Kevin wrote: > > From: Alex Williamson <alex.williamson@redhat.com> > > Sent: Friday, December 2, 2022 5:10 AM > > > > > Otherwise what this patch does looks better IMHO: > > > > > > ret = device_register(&dev); > > > if (ret) { > > > put_device(&dev); > > > goto err1; > > > } > > > > > > ... > > > > > > return 0; > > > > > > err2: > > > device_unregister(&dev); > > > err1: > > > earlier_unwind(); > > > > > > > This is essentially what was originally proposed. It could also be > > called a "mixed model", implementing part of the unwind in the > > error > > branch before jumping to the common unwind. As demonstrated below, > > every current vfio driver calling device_register() follows a > > similar > > goto unwind stack as found in the sample drivers, which makes it > > trivially easy to split the device_unregister() call and add a goto > > target in between. > > OK. Actually looking into other users of device_register() I can > see both schemes are used, so... > > > > > Either way, they're equivalent and I'll take whichever version > > addresses all the vfio related use cases and gets acks from their > > maintainers. Thanks, > > ... agree either way is fine. Since your version fixes all vfio > cases, > feel free to include: > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > > > > Alex > > > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c > > b/drivers/s390/cio/vfio_ccw_drv.c > > index c2a65808605a..54aba7cceb33 100644 > > --- a/drivers/s390/cio/vfio_ccw_drv.c > > +++ b/drivers/s390/cio/vfio_ccw_drv.c > > @@ -199,8 +199,9 @@ static int vfio_ccw_sch_probe(struct subchannel > > *sch) > > return 0; > > > > out_unreg: > > - device_unregister(&parent->dev); > > + device_del(&parent->dev); > > out_free: > > + put_device(&parent->dev); > > dev_set_drvdata(&sch->dev, NULL); > > return ret; > > } Ugh, sorry. I had this partially right at one point but missed this scenario when fixing the parts I had wrong. This is fine by me, thanks for the fixup: Reviewed-by: Eric Farman <farman@linux.ibm.com> > > diff --git a/drivers/s390/crypto/vfio_ap_drv.c > > b/drivers/s390/crypto/vfio_ap_drv.c > > index f43cfeabd2cc..997b524bdd2b 100644 > > --- a/drivers/s390/crypto/vfio_ap_drv.c > > +++ b/drivers/s390/crypto/vfio_ap_drv.c > > @@ -122,7 +122,7 @@ static int vfio_ap_matrix_dev_create(void) > > return 0; > > > > matrix_drv_err: > > - device_unregister(&matrix_dev->device); > > + device_del(&matrix_dev->device); > > matrix_reg_err: > > put_device(&matrix_dev->device); > > matrix_alloc_err: > > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio- > > mdev/mbochs.c > > index 8b5a3a778a25..e54eb752e1ba 100644 > > --- a/samples/vfio-mdev/mbochs.c > > +++ b/samples/vfio-mdev/mbochs.c > > @@ -1430,7 +1430,7 @@ static int __init mbochs_dev_init(void) > > > > ret = device_register(&mbochs_dev); > > if (ret) > > - goto err_class; > > + goto err_put; > > > > ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, > > &mbochs_driver, > > mbochs_mdev_types, > > @@ -1441,8 +1441,9 @@ static int __init mbochs_dev_init(void) > > return 0; > > > > err_device: > > - device_unregister(&mbochs_dev); > > -err_class: > > + device_del(&mbochs_dev); > > +err_put: > > + put_device(&mbochs_dev); > > class_destroy(mbochs_class); > > err_driver: > > mdev_unregister_driver(&mbochs_driver); > > diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c > > index 721fb06c6413..e8400fdab71d 100644 > > --- a/samples/vfio-mdev/mdpy.c > > +++ b/samples/vfio-mdev/mdpy.c > > @@ -717,7 +717,7 @@ static int __init mdpy_dev_init(void) > > > > ret = device_register(&mdpy_dev); > > if (ret) > > - goto err_class; > > + goto err_put; > > > > ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, > > &mdpy_driver, > > mdpy_mdev_types, > > @@ -728,8 +728,9 @@ static int __init mdpy_dev_init(void) > > return 0; > > > > err_device: > > - device_unregister(&mdpy_dev); > > -err_class: > > + device_del(&mdpy_dev); > > +err_put: > > + put_device(&mdpy_dev); > > class_destroy(mdpy_class); > > err_driver: > > mdev_unregister_driver(&mdpy_driver); > > diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c > > index 3c2a421b9b69..e887de672c52 100644 > > --- a/samples/vfio-mdev/mtty.c > > +++ b/samples/vfio-mdev/mtty.c > > @@ -1330,7 +1330,7 @@ static int __init mtty_dev_init(void) > > > > ret = device_register(&mtty_dev.dev); > > if (ret) > > - goto err_class; > > + goto err_put; > > > > ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, > > &mtty_driver, mtty_mdev_types, > > @@ -1340,8 +1340,9 @@ static int __init mtty_dev_init(void) > > return 0; > > > > err_device: > > - device_unregister(&mtty_dev.dev); > > -err_class: > > + device_del(&mtty_dev.dev); > > +err_put: > > + put_device(&mtty_dev.dev); > > class_destroy(mtty_dev.vd_class); > > err_driver: > > mdev_unregister_driver(&mtty_driver); >
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index 117a8d799f71..1c47672be815 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -1430,8 +1430,10 @@ static int __init mbochs_dev_init(void) dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); ret = device_register(&mbochs_dev); - if (ret) + if (ret) { + put_device(&mbochs_dev); goto err_class; + } ret = mdev_register_parent(&mbochs_parent, &mbochs_dev, &mbochs_driver, mbochs_mdev_types, diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index 946e8cfde6fd..bfb93eaf535b 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -717,8 +717,10 @@ static int __init mdpy_dev_init(void) dev_set_name(&mdpy_dev, "%s", MDPY_NAME); ret = device_register(&mdpy_dev); - if (ret) + if (ret) { + put_device(&mdpy_dev); goto err_class; + } ret = mdev_register_parent(&mdpy_parent, &mdpy_dev, &mdpy_driver, mdpy_mdev_types, diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index e72085fc1376..dddb0619846c 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -1330,8 +1330,10 @@ static int __init mtty_dev_init(void) dev_set_name(&mtty_dev.dev, "%s", MTTY_NAME); ret = device_register(&mtty_dev.dev); - if (ret) + if (ret) { + put_device(&mtty_dev.dev); goto err_class; + } ret = mdev_register_parent(&mtty_dev.parent, &mtty_dev.dev, &mtty_driver, mtty_mdev_types,