Re: [PATCH 2/3] x86: tss: Eliminate fragile calculation of TSS segment limit

[Josh Triplett <josh@joshtriplett.org>]

On Wed, Oct 30, 2013 at 03:22:33PM -0700, Kees Cook wrote:
> On Mon, Oct 21, 2013 at 7:34 PM, Josh Triplett <josh@joshtriplett.org> wrote:
> > __set_tss_desc has a complex calculation of the TSS segment limit,
> > duplicating the quirky details of the I/O bitmap array length, and
> > requiring a complex comment to explain.  Replace that calculation with a
> > simpler one based on the offsetof the "stack" field that follows the
> > array.
> >
> > That then removes the last use of IO_BITMAP_OFFSET, so delete it.
> >
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > ---
> >  arch/x86/include/asm/desc.h      | 11 +----------
> >  arch/x86/include/asm/processor.h |  3 ++-
> >  2 files changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> > index b90e5df..17ac92f 100644
> > --- a/arch/x86/include/asm/desc.h
> > +++ b/arch/x86/include/asm/desc.h
> > @@ -177,16 +177,7 @@ static inline void __set_tss_desc(unsigned cpu, unsigned int entry, void *addr)
> >         struct desc_struct *d = get_cpu_gdt_table(cpu);
> >         tss_desc tss;
> >
> > -       /*
> > -        * sizeof(unsigned long) coming from an extra "long" at the end
> > -        * of the iobitmap. See tss_struct definition in processor.h
> > -        *
> > -        * -1? seg base+limit should be pointing to the address of the
> > -        * last valid byte
> 
> I think it might be better to keep at least a minimal comment near the
> TSS_LIMIT declaration, just to explain the "-1" part, which is not
> entirely obvious from just reading the code.

Fair enough; I've added an appropriate comment next to TSS_LIMIT, and
I'll include that in PATCHv2, which I'll send out as soon as I see
any feedback on patch 3/3.

- Josh Triplett