public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Shiju Jose <shiju.jose@huawei.com>
Cc: "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mchehab@kernel.org" <mchehab@kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"james.morse@arm.com" <james.morse@arm.com>,
	Linuxarm <linuxarm@huawei.com>, Robert Richter <rric@kernel.org>
Subject: Re: [PATCH 1/1] EDAC/ghes: Fix for NULL pointer dereference in ghes_edac_register()
Date: Fri, 11 Sep 2020 18:48:17 +0200	[thread overview]
Message-ID: <20200911164817.GA19320@zn.tnic> (raw)
In-Reply-To: <cd947c4ec6044521a92e2cc39eae5406@huawei.com>

On Thu, Aug 27, 2020 at 02:02:27PM +0000, Shiju Jose wrote:
> I tested with your changes and it fixes the issue.  I will send v2.

Btw, I don't know how it managed to work on your machine because even
with this patch, it isn't all fixed because num_dimms needs to be
cleared too, see here:

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 11 Sep 2020 12:55:55 +0200
Subject: [PATCH] EDAC/ghes: Clear scanned data on unload

Commit

  b972fdba8665 ("EDAC/ghes: Fix NULL pointer dereference in ghes_edac_register()")

didn't clear all the information from the scanned system and, more
specifically, left ghes_hw.num_dimms to its previous value. On a
second load (CONFIG_DEBUG_TEST_DRIVER_REMOVE=y), the driver would use
the leftover num_dimms value which is not 0 and thus the 0 check in
enumerate_dimms() will get bypassed and it would go directly to the
pointer deref:

  d = &hw->dimms[hw->num_dimms];

which is, of course, NULL:

  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0002) - not-present page
  PGD 0 P4D 0
  Oops: 0002 [#1] PREEMPT SMP
  CPU: 7 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4+ #7
  Hardware name: GIGABYTE MZ01-CE1-00/MZ01-CE1-00, BIOS F02 08/29/2018
  RIP: 0010:enumerate_dimms.cold+0x7b/0x375

Reset the whole ghes_hw on driver unregister so that no stale values are
used on a second system scan.

Fixes: b972fdba8665 ("EDAC/ghes: Fix NULL pointer dereference in ghes_edac_register()")
Cc: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/edac/ghes_edac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index a6b9c0b2a15c..eb6034a6fbbb 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -632,6 +632,7 @@ void ghes_edac_unregister(struct ghes *ghes)
 	mutex_lock(&ghes_reg_mutex);
 
 	system_scanned = false;
+	memset(&ghes_hw, 0, sizeof(struct ghes_hw_desc));
 
 	if (!refcount_dec_and_test(&ghes_refcount))
 		goto unlock;
-- 
2.21.0

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2020-09-11 16:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25 13:01 [PATCH 1/1] EDAC/ghes: Fix for NULL pointer dereference in ghes_edac_register() Shiju Jose
2020-08-26  8:52 ` Borislav Petkov
2020-08-27 14:02   ` Shiju Jose
2020-09-11 16:48     ` Borislav Petkov [this message]
2020-09-11 16:49       ` [PATCH] EDAC/ghes: Check whether the driver is on the safe list correctly Borislav Petkov
2020-09-15 10:54       ` [PATCH 1/1] EDAC/ghes: Fix for NULL pointer dereference in ghes_edac_register() Shiju Jose
2020-09-15 12:25         ` Borislav Petkov

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=20200911164817.GA19320@zn.tnic \
    --to=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=mchehab@kernel.org \
    --cc=rric@kernel.org \
    --cc=shiju.jose@huawei.com \
    --cc=tony.luck@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox