All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Richter <rrichter@marvell.com>
To: "john.garry@huawei.com" <john.garry@huawei.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Borislav Petkov <bp@alien8.de>, Tony Luck <tony.luck@intel.com>
Cc: "huangming23@huawei.com" <huangming23@huawei.com>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxarm@huawei.com" <linuxarm@huawei.com>,
	Robert Richter <rrichter@marvell.com>,
	"tanxiaofei@huawei.com" <tanxiaofei@huawei.com>,
	"wanghuiqiang@huawei.com" <wanghuiqiang@huawei.com>
Subject: [PATCH] EDAC/ghes: Do not warn when incrementing refcount on 0
Date: Thu, 21 Nov 2019 21:36:57 +0000	[thread overview]
Message-ID: <20191121213628.21244-1-rrichter@marvell.com> (raw)
In-Reply-To: <4ff7631f-fbb7-e45f-87dd-9223beca4da7@huawei.com>

Following warning from the refcount framework is seen during ghes
initialization:

 EDAC MC0: Giving out device to module ghes_edac.c controller ghes_edac: DEV ghes (INTERRUPT)
 ------------[ cut here ]------------
 refcount_t: increment on 0; use-after-free.
 WARNING: CPU: 36 PID: 1 at lib/refcount.c:156 refcount_inc_checked+0x44/0x50
[...]
 Call trace:
  refcount_inc_checked+0x44/0x50
  ghes_edac_register+0x258/0x388
  ghes_probe+0x28c/0x5f0

It warns if the refcount is incremented from zero. This warning is
reasonable as a kernel object is typically created with a refcount of
one and freed once the refcount is zero. Afterwards the object would
be "used-after-free".

For ghes the refcount is initialized with zero, and that is why this
message is seen when initializing the first instance. However,
whenever the refcount is zero, the device will be allocated and
registered. Since the ghes_reg_mutex protects the refcount and
serializes allocation and freeing of ghes devices, a use-after-free
cannot happen here.

Instead of using refcount_inc() for the first instance, use
refcount_set(). This can be used here because the refcount is zero at
this point and can not change due to its protection by the mutex.

Reported-by: John Garry <john.garry@huawei.com>
Tested-by: John Garry <john.garry@huawei.com>
Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 drivers/edac/ghes_edac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 47f4e7f90ef0..b99080d8a10c 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -556,8 +556,8 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
 	ghes_pvt = pvt;
 	spin_unlock_irqrestore(&ghes_lock, flags);
 
-	/* only increment on success */
-	refcount_inc(&ghes_refcount);
+	/* only set on success */
+	refcount_set(&ghes_refcount, 1);
 
 unlock:
 	mutex_unlock(&ghes_reg_mutex);
-- 
2.20.1


  reply	other threads:[~2019-11-21 21:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-14 15:18 edac KASAN warning in experimental arm64 allmodconfig boot John Garry
2019-10-14 16:09 ` Borislav Petkov
2019-10-14 16:44   ` John Garry
2019-10-14 16:15 ` James Morse
2019-10-14 16:56   ` John Garry
2019-10-14 16:57     ` Borislav Petkov
2019-11-21 12:34 ` linuxnext-2019119 edac warns (was Re: edac KASAN warning in experimental arm64 allmodconfig boot) John Garry
2019-11-21 14:23   ` Robert Richter
2019-11-21 15:23     ` John Garry
2019-11-21 21:36       ` Robert Richter [this message]
2019-11-22  9:01         ` [PATCH] EDAC/ghes: Do not warn when incrementing refcount on 0 Borislav Petkov
2019-11-26  9:57           ` John Garry
2019-11-22 11:28       ` linuxnext-2019119 edac warns (was Re: edac KASAN warning in experimental arm64 allmodconfig boot) Robert Richter
2019-11-26  9:59         ` John Garry
2019-11-27 17:07           ` linuxnext-2019127 " John Garry
2019-11-27 20:54             ` Robert Richter
2019-11-28 11:02               ` linuxnext-20191127 " John Garry
2019-11-28 16:44                 ` Borislav Petkov
2019-11-28 21:12             ` linuxnext-2019127 " Robert Richter
2019-12-02 10:23               ` John Garry
2019-12-02 11:46                 ` Robert Richter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191121213628.21244-1-rrichter@marvell.com \
    --to=rrichter@marvell.com \
    --cc=bp@alien8.de \
    --cc=huangming23@huawei.com \
    --cc=james.morse@arm.com \
    --cc=john.garry@huawei.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=mchehab@kernel.org \
    --cc=tanxiaofei@huawei.com \
    --cc=tony.luck@intel.com \
    --cc=wanghuiqiang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.