linux-kernel - Re: [PATCH 10/10] MCE: Add Action-Required support

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]

Message-ID: <BANLkTim2CoU4aQZipXWn9gW5TexCrtoEJA@mail.gmail.com>
Date:	Fri, 10 Jun 2011 14:00:42 -0700
From:	Tony Luck <tony.luck@...el.com>
To:	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Borislav Petkov <bp@...64.org>,
	linux-kernel@...r.kernel.org, "Huang, Ying" <ying.huang@...el.com>,
	Avi Kivity <avi@...hat.com>
Subject: Re: [PATCH 10/10] MCE: Add Action-Required support

2011/6/10 Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>:
>> +static int kernel_ar_recoverable(struct mce *m, int tolerant)
>> +{
>> +     if (tolerant >= 2)
>> +             return MCE_AR_SEVERITY;
>> +     if (!(m->mcgstatus & MCG_STATUS_EIPV) || !m->ip)
>> +             return MCE_PANIC_SEVERITY;
>> +     if (search_exception_tables(m->ip))
>> +             return MCE_AR_SEVERITY;
>> +     return MCE_PANIC_SEVERITY;
>> +}
>> +
>
> You said "Just handle errors in user mode for now." but ...?

Oops. I dropped the kernel handling bits from mce.c, but forget to
clean them out of mce-severity.c

Will fix.

>> +     if (!mce_usable_address(m))
>> +             mce_panic("No address for Action-Required Machine Check",
>> +                       m, msg);
>> +     if (!(m->mcgstatus & MCG_STATUS_EIPV))
>> +             mce_panic("No EIPV for Action-Required Machine Check",
>> +                       m, msg);
>
> When can this happen?

Probably never (or only if there was a chip bug. AR errors should
provide addresses
(otherwise there is no way to perform any useful "action").

> Why not create new severity {PANIC, "Action Required: but No EIPV", ...}
> in severity table?

Very good idea.

>> +static void ar_fallback(struct task_struct *me, unsigned long pfn)
>> +{
>> +     if (signal_pending(me) && sigismember(&me->pending.signal, SIGBUS))
>> +             return;
>
> Is it safe for _AR if SIGBUS is pending but blocked?
> I think force_sig() is reasonable for such situation.

Agreed - it is not safe if the signal is blocked.  force_sig() sounds better.

>> +     if (worst == MCE_AR_SEVERITY) {
>> +             mce_action_required(&m, msg, regs);
>
> Comprehensible name would be appreciated, e.g.:
>                mce_request_dpc_for_action_required(pfn);

I'm always in the mood for better names ... but I'm not sure what
the "dpc" in this name is trying to tell me.

> And if we cannot request context for recovery, it is better to suppress
> trailing attempts, e.g. before mce_end():
>
>        if (!no_way_out && severity == MCE_AR_SEVERITY) {
>                err = mce_request_dpc_for_action_required(pfn);
>                if (err) {
>                        atomic_inc(&global_nwo);
>                        severity = MCE_PANIC_SEVERITY;  /* escalated */
>                }
>        }

Agreed - if we can't perform the recovery action, we should escalate
the severity.

-Tony
--
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/