lkml.org 
[lkml]   [2019]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch V2 08/16] x86/ioperm: Add bitmap sequence number
On Tue, 12 Nov 2019, Andy Lutomirski wrote:
> On Mon, Nov 11, 2019 at 2:35 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> > /*
> > + * The bitmap pointer and the sequence number of the last active
> > + * bitmap. last_bitmap cannot be dereferenced. It's solely for
> > + * comparison.
> > + */
> > + struct io_bitmap *last_bitmap;
> > + u64 last_sequence;
> > +
> > + /*
> > * Store the dirty size of the last io bitmap offender. The next
> > * one will have to do the cleanup as the switch out to a non io
> > * bitmap user will just set x86_tss.io_bitmap_base to a value
>
> Why is all this stuff in the TSS? Would it make more sense in a
> percpu variable tss_state? By putting it in the TSS, you're putting
> it in cpu_entry_area, which is at least a bit odd if not an actual
> security problem.
>
> And why do you need a last_bitmap pointer? I thin that comparing just
> last_sequence should be enough. Keeping last_bitmap around at all is
> asking for trouble, since it might point to freed memory.

The bitmap pointer is pointless as I said in an earlier reply to Peter. It
will go away. The sequence number and the dirty size are surely not a
problem leakage wise, but yes, we could put it into a per cpu variable as
well. Not sure whether it buys much.

> > - memcpy(tss->io_bitmap_bytes, iobm->bitmap_bytes,
> > - max(tss->io_bitmap_prev_max, iobm->io_bitmap_max));
> > + if (tss->last_bitmap != iobm ||
> > + tss->last_sequence != iobm->sequence)
>
> As above, I think this could just be if (tss->last_sequence !=
> iobm->sequence) or even if (this_cpu_read(cpu_tss_state.iobm_sequence)
> != iobm->sequence).

Already fixed as per Peter's comments.

Thanks,

tglx

\
 
 \ /
  Last update: 2019-11-12 18:11    [W:0.090 / U:1.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site