linux-kernel - Re: amd64_edac making improper assumptions?

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] [day] [month] [year] [list]

Message-ID: <20090918133622.GB25309@aftab>
Date:	Fri, 18 Sep 2009 15:36:22 +0200
From:	Borislav Petkov <borislav.petkov@....com>
To:	Jan Beulich <JBeulich@...ell.com>
CC:	dougthompson@...ssion.com, linux-kernel@...r.kernel.org
Subject: Re: amd64_edac making improper assumptions?

Hi Jan,

On Fri, Sep 18, 2009 at 02:02:28PM +0100, Jan Beulich wrote:
> Doug, Borislav,
> 
> the sizing of pvt_lookup[] and mci_lookup[] is done based on a config
> setting (CONFIG_NODES_SHIFT), and the indexing happens using the
> raw value read from hardware without any bounds checking. Hence,
> running a kernel with e.g. !CONFIG_NUMA on a multi-socket system
> would happily access other than the first and only array element.

Good catch, will fix, thanks. See below for longer explanation.

> Likewise, the use of rdmsr_on_cpu() doesn't seem to do what is
> intended when !CONFIG_NUMA.
> 
> Also, assuming you can use cpumask_of_node() directly on the
> node ID read from hardware seems bogus (even in general, but
> namely again when MAX_NUMNODES is less than the actual number
> of nodes).

originally, the driver was compiling in NUMA for facilities like
cpumask_of_node() et al for all per-node stuff but Ingo raised a concern
that the NUMA dependency shouldn't be needed. Therefore, we needed some
alternative facilities which, for example, can give you all the cores
on a node so that you can init a driver instance per node and thus per
memory controller.

Well, Andreas did prepare a bunch of generic cpu topology patches which
handle also MCM processors (e.g. having more than one internal node) and
currently the per-CPU cpu_llc_id var contains the node id of a cpu. They
went upstream last week and I'm waiting for another patch in -tip to go
in before I send my local queue here which removes the cpumask_of_node()
usage and all the rest of NUMA stuff completely.

Hope that makes it more clear and thanks for letting us know, we're on
it :).

-- 
Regards/Gruss,
Boris.

Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/