From: Shiju Jose <shiju.jose@huawei.com>
To: Borislav Petkov <bp@alien8.de>
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: Tue, 15 Sep 2020 10:54:33 +0000 [thread overview]
Message-ID: <66270fbc43fc4bd686b638586710d9dd@huawei.com> (raw)
In-Reply-To: <20200911164817.GA19320@zn.tnic>
Hi Boris,
Sorry for the delay.
>-----Original Message-----
>From: Borislav Petkov [mailto:bp@alien8.de]
>Sent: 11 September 2020 17:48
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org;
>mchehab@kernel.org; tony.luck@intel.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()
>
>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:
I debug with adding more logs.
I found that in our platform hw->num_dimms was 32 when called ghes_edac_register() second time
when probe a new ghes instance, the check !(hw->num_dimms % 16) in the enumerate_dimms() passed and
it allocated memory for hw->dimms. Thus it did not fail with NULL pointer dereference in ghes_edac_register().
With the your new fix hw->num_dimms reset to 0.
>
>---
>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
Thanks,
Shiju
next prev parent reply other threads:[~2020-09-15 10:54 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
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 ` Shiju Jose [this message]
2020-09-15 12:25 ` [PATCH 1/1] EDAC/ghes: Fix for NULL pointer dereference in ghes_edac_register() 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=66270fbc43fc4bd686b638586710d9dd@huawei.com \
--to=shiju.jose@huawei.com \
--cc=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=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;
as well as URLs for NNTP newsgroup(s).