linux-kernel - Re: [PATCH -v2] x86: MCE: Re-implement MCE log ring buffer as per-CPU ring buffer

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, 29 Apr 2009 09:31:49 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andi Kleen <ak@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -v2] x86: MCE: Re-implement MCE log ring buffer as
 per-CPU ring buffer

On Tue, 2009-04-28 at 18:21 +0800, Andi Kleen wrote:
> Huang Ying <ying.huang@...el.com> writes:
> >
> > ChangeLog:
> >
> > v2:
> >
> > - Use alloc_percpu() to allocate per_cpu mcelog buffer
> 
> Sorry, why didn't you just use DEFINE_PER_CPU ? That should work
> as well and will be shorter.

OK. I can do that.

> Another thing I noticed. the "MACHINECHECK" signature was originally
> for crash dump tools to find the log. If you change the format
> you should change it to MACHINECHEC2 or so.

Oh, thanks, I will change this.

> > +	size_t usize_limit;
> > +
> > +	/* Too large user buffer size may cause system not response */
> > +	usize_limit = num_possible_cpus() * MCE_LOG_LEN * sizeof(struct mce);
> > +	if (usize > usize_limit)
> > +		usize = usize_limit;
> 
> Did you ever track down what happens here? I still find it worrying

Maybe the comment is a little confusing. What we do is limit the size of
user buffer size to prevent system from not response. If the user buffer
size is limited, the time taken by mce_read() can be reasonable, and
will not cause system not to response.

But maybe num_possible_cpus() * MCE_LOG_LEN is not a good upper limit,
because possible cpus can be fairly large. Maybe we can add another
limit, something as follow:

#define MCE_READ_RECORD_LIMIT	512

/*
 * Too large user buffer size may cause system not response, so constrain
 * the effective size of user buffer
 */
usize_limit = num_possible_cpus() * MCE_LOG_LEN * sizeof(struct mce);
if (usize_limit > MCE_RECOARD_LIMIT * sizeof(struct mce))
	usize_limit = MCE_RECOARD_LIMIT * sizeof(struct mce);
if (usize > usize_limit)
	usize = usize_limit;

Best Regards,
Huang Ying


Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)