linux-kernel - [PATCH AUTOSEL 5.4 330/350] EDAC/ghes: Do not warn when incrementing refcount on 0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Dec 2019 16:07:15 -0500
From:   Sasha Levin <sashal@...nel.org>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:     Robert Richter <rrichter@...vell.com>,
        John Garry <john.garry@...wei.com>,
        Borislav Petkov <bp@...e.de>, huangming23@...wei.com,
        James Morse <james.morse@....com>, linuxarm@...wei.com,
        linux-edac <linux-edac@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        tanxiaofei@...wei.com, Tony Luck <tony.luck@...el.com>,
        wanghuiqiang@...wei.com, Sasha Levin <sashal@...nel.org>
Subject: [PATCH AUTOSEL 5.4 330/350] EDAC/ghes: Do not warn when incrementing refcount on 0

From: Robert Richter <rrichter@...vell.com>

[ Upstream commit 16214bd9e43a31683a7073664b000029bba00354 ]

The 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
 [...]
  Call trace:
   refcount_inc_checked
   ghes_edac_register
   ghes_probe
   ...

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.

Fixes: 23f61b9fc5cc ("EDAC/ghes: Fix locking and memory barrier issues")
Reported-by: John Garry <john.garry@...wei.com>
Signed-off-by: Robert Richter <rrichter@...vell.com>
Signed-off-by: Borislav Petkov <bp@...e.de>
Tested-by: John Garry <john.garry@...wei.com>
Cc: <huangming23@...wei.com>
Cc: James Morse <james.morse@....com>
Cc: <linuxarm@...wei.com>
Cc: linux-edac <linux-edac@...r.kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: <tanxiaofei@...wei.com>
Cc: Tony Luck <tony.luck@...el.com>
Cc: <wanghuiqiang@...wei.com>
Link: https://lkml.kernel.org/r/20191121213628.21244-1-rrichter@marvell.com
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
 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 1858baa96211b..523dd56a798c9 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -572,8 +572,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