linux-kernel - RE: [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove the dependency on ghes

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 17 Aug 2022 01:39:32 +0000
From:   "Kani, Toshi" <toshi.kani@....com>
To:     Justin He <Justin.He@....com>, Borislav Petkov <bp@...en8.de>
CC:     Ard Biesheuvel <ardb@...nel.org>, Len Brown <lenb@...nel.org>,
        James Morse <James.Morse@....com>,
        Tony Luck <tony.luck@...el.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Robert Richter <rric@...nel.org>,
        Robert Moore <robert.moore@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "devel@...ica.org" <devel@...ica.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Shuai Xue <xueshuai@...ux.alibaba.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        nd <nd@....com>, "stable@...nel.org" <stable@...nel.org>
Subject: RE: [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove the
 dependency on ghes

On Monday, August 15, 2022 8:20 PM, Justin He wrote:
> I assume that all those edac drivers which used owner checking are
> impacted, right? So the impacted list should be:
> drivers/edac/pnd2_edac.c:1532:  if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
> drivers/edac/sb_edac.c:3513:    if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
> drivers/edac/amd64_edac.c:4333: if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
> drivers/edac/i10nm_base.c:552:  if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
> drivers/edac/skx_base.c:657:    if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
> drivers/edac/igen6_edac.c:1275: if (owner && strncmp(owner,
> EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))

Yes, but the impact is not necessarily limited to these modules.

The current model works as follows:
 1. ghes_probe() calling ghes_edac_register() sets the owner to
ghes_edac.
 2. chipset-specific edac drivers checks the owner.
 3. edac_mc_add_mc_with_groups() also checks the owner.

Hence, check #3 enforces #1 (ghes_edac) even if there is an edac
driver w/o check #2.  I do not know if such case exists though.

> Furthermore, should I totally remove the owner check in above driver
> XX_init()? Because after the new helper ghes_get_device() checking, those
> drivers can't be initialized after ghes_edac is loaded. If ghes_edac is NOT
> loaded, the edac_mc_owner check in edac_mc_add_mc_with_groups() can
> guarantee that there is only one regular edac driver.
> 
> What do you think of it?

I think a new check with ghes_get_device() can replace the owner check
in xx_init().

Thanks,
Toshi