Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1801281232130.2126@nanos>
Date: Sun, 28 Jan 2018 12:36:33 +0100 (CET)
From: Thomas Gleixner <tglx@...utronix.de>
To: Ingo Molnar <mingo@...nel.org>
cc: Dan Williams <dan.j.williams@...el.com>, linux-arch@...r.kernel.org, 
    Cyril Novikov <cnovikov@...x.com>, kernel-hardening@...ts.openwall.com, 
    Peter Zijlstra <peterz@...radead.org>, 
    Catalin Marinas <catalin.marinas@....com>, x86@...nel.org, 
    Will Deacon <will.deacon@....com>, Russell King <linux@...linux.org.uk>, 
    Ingo Molnar <mingo@...hat.com>, gregkh@...uxfoundation.org, 
    "H. Peter Anvin" <hpa@...or.com>, torvalds@...ux-foundation.org, 
    alan@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 02/12] array_idx: sanitize speculative array
 de-references

On Sun, 28 Jan 2018, Ingo Molnar wrote:
> * Dan Williams <dan.j.williams@...el.com> wrote:
> > +
> > +#ifndef __NOSPEC_H__
> > +#define __NOSPEC_H__

_LINUX_NOSPEC_H

We have an established practice for those header guards...

> > +/*
> > + * When idx is out of bounds (idx >= sz), the sign bit will be set.
> > + * Extend the sign bit to all bits and invert, giving a result of zero
> > + * for an out of bounds idx, or ~0UL if within bounds [0, sz).
> > + */
> > +#ifndef array_idx_mask
> > +static inline unsigned long array_idx_mask(unsigned long idx, unsigned long sz)
> > +{
> > +	/*
> > +	 * Warn developers about inappropriate array_idx usage.
> > +	 *
> > +	 * Even if the cpu speculates past the WARN_ONCE branch, the
> 
> s/cpu/CPU
> 
> > +	 * sign bit of idx is taken into account when generating the
> > +	 * mask.
> > +	 *
> > +	 * This warning is compiled out when the compiler can infer that
> > +	 * idx and sz are less than LONG_MAX.
> 
> Please use 'idx' and 'sz' in quotes, to make sure they stand out more in free 
> flowing comment text. Also please use '()' to denote functions/methods.
> 
> I.e. something like:
> 
> 	 * Warn developers about inappropriate array_idx() usage.
> 	 *
> 	 * Even if the CPU speculates past the WARN_ONCE() branch, the
> 	 * sign bit of 'idx' is taken into account when generating the
> 	 * mask.
> 	 *
> 	 * This warning is compiled out when the compiler can infer that
> 	 * 'idx' and 'sz' are less than LONG_MAX.

I rather prefer to have a proper kernel doc instead of the comment above
the function and then use @idx and @sz in the code comments.

Thanks,

	tglx

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.