linux-kernel - Re: [PATCH] RAS/CEC: Add debugfs switch to disable at run time

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:   Mon, 22 Apr 2019 10:44:15 -0700
From:   "Luck, Tony" <tony.luck@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] RAS/CEC: Add debugfs switch to disable at run time

On Mon, Apr 22, 2019 at 07:15:32PM +0200, Borislav Petkov wrote:
> On Mon, Apr 22, 2019 at 03:59:16PM +0000, Luck, Tony wrote:
> > > Err, this all sounds to me like the storm detection code should
> > > *automatically* disable the CEC in such cases, I'd say.
> > 
> > Sounds good. But we should distinguish storms that have many different
> > addresses from storms that just ping a few addresses.  CEC will see counts
> > hit the threshold in the latter case, but it might not be able to take the pages
> > offline (because they are locked, or in-use by kernel).
> > 
> > So I think the change might be to the return value from NOTIFY_STOP to NOTIFY_DONE
> > ... but only if we are in the middle of a storm AND the CEC array is full.
> 
> Well, regardless of this specific use case, isn't that a generic enough
> action that we should do always? I mean, the aspect of falling back to
> logging to external agent.

Yes. Automating this would be a very good idea.

> However, currently we don't signal that the CEC is full - we simply
> remove the LRU element in cec_add_elem() before we insert the new one.
> 
> We can either return a specific retval to say, CEC is full and we had to
> delete an elem or we can add a cec_is_full() accessor...

A lot depends on why the CEC is full, and which entry is being
deleted to make room.

In the case of many errors at different addresses we are deleting
the entry with the lowest count. But all of the entries have low
counts because we are just thrashing the array with many different
addresses. In this situation a warning would be helpful.

But in the case where the system has been up for months and
we very slowly accumlated logs of bit flips. The periodic
spring cleaning means they all have generation "00", but
we never actually drop an old entry because of age. In this
case dropping one entry to make space for a new one is fine
and doesn't need any action.

Perhaps we can distinguish the cases by the generation? If
we are dropping an entry that was recently added, then it
will still have generation "11" (or at least not "00").
Use that to trigger an action?

-Tony