linux-kernel - Re: [PATCH 3/3] EDAC, ghes: Make it a proper module

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:   Wed, 2 Aug 2017 05:18:50 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Kani, Toshimitsu" <toshi.kani@....com>
Cc:     "mchehab@...radead.org" <mchehab@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>
Subject: Re: [PATCH 3/3] EDAC, ghes: Make it a proper module

On Wed, Aug 02, 2017 at 12:19:29AM +0000, Kani, Toshimitsu wrote:
> 1. Device-probing-logic should belong to a driver, and should remain
> private to a driver.  When we add the white-list, it should be added to
> ghes_edac.

Nonsense. There are a lot of examples where driver probing depends on
outside modalities like built-in quirks and such.

> 2. ghes_edac is an extension to the ghes driver as they both are
> specific to ghes.  ghes_edac is merely ghes driver's edac error-
> reporting wrapper than an independent edac driver.  It looks OK to let
> ghes_edac get registered as part of ghes_probe() and leave it as an
> unconventional edac driver.

Except that GHES wants to report into the EDAC infrastructure so it
better has a wrapper for it.

One of the directions I explored when looking at this is to stick
ghes_edac functionality into ghes.c or so and make it completely
independent from EDAC. Would've been much cleaner.

> 3. EDAC does not have its managed probe-chain.  All edac drivers are
> called from module_init list.  They independently probe the hardware
> and get unloaded when not needed.  The core edac is simply a set of
> library to them.  I think it's good to keep them independent, and not
> to introduce a new central mechanism for a special case like ghes_edac.

They're independent because before GHES we needed to load one driver per
system. Until the bolted-on thing came. And it is bolted on because the
already overwhelmed firmware decided to do error reporting too.

So the only real reason why I'm fine with keeping the current situation
is the whitelist. Because then, we can at least control what loads and
what not.

But then we need:

1. A clean mechanism for the platform drivers to query whether another
agent is loaded (ghes_edac) and not do any probing then.

2. ghes_edac needs to drop that multiple probing thing as its
dmi_walk(ghes_edac_count_dimms, &num_dimm) already probes *all* DIMMs on
the system so no need to do that multiple times.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--