Messages in this thread | | | Date | Wed, 22 Apr 2015 17:32:45 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 01/16] x86, fpu: wrap get_xsave_addr() to make it safer |
| |
On 04/22, Borislav Petkov wrote: > > On Wed, Apr 22, 2015 at 03:16:18PM +0200, Oleg Nesterov wrote: > > I agree, tsk_used_math(tsk) looks better, simpy because we have this > > argument. > > > > But this "tsk" should be always current, otherwise this code is wrong > > This is exactly what I'm asking: is that always the case?...
I can't look at these patches now, but iirc - yes. The caller is either prctl() or exception. Dave will correct me.
Otherwise, once again, this code is simply buggy. So the comment should probably explain this.
> > > Because used_math() is looking at current, maybe even in > > > preemption-enabled paths - I'm eyeing task_get_bounds_dir() - and > > > that current might get changed from under us and it might happen that > > > current != tsk. Yes, no? > > > > Not sure I understand... "current" can't change from under us? > > ... I'm not sure all tsk_get_xsave_field() callers disable preemption. > If not, then current can change from under us...
How? I am certainly missing you point... OK, please forge about FPU. Consider this code:
tsk = current; for (;;) BUG_ON(tsk != current);
it doesn't need to disable preemption. We do not care if CPU switches to another thread, even if this thread executes the same code. Because its tsk/current will differ, but "tsk == current" will be still true.
Could you please spell?
> > Even if this CPU switches to another thread which executes the same code, > > that thread will obviously see another "current", but its "tsk" variable > > will still match its "current". > > Well, we want to see if @tsk used math, not necessarily if current used > math, especially if it is another task, right?
See above... used_math() should be correct because we know that tsk==current, but I agree that tsk_used_math(tsk) looks better.
> I read tsk_get_xsave_field(@tsk, ) as give me the xsave field of @tsk > but doing used_math() we're querying current and I'm not sure > > tsk == current > > in all the call sites of tsk_get_xsave_field().
Yes, the name/comment looks confusing a bit, as if you can use it when tsk != current...
Oleg.
| |