On Wed, Dec 07 2016, Olga Kornievskaia wrote:
>>>
>>> Agreed. This is a problem.
>>>
>>> Doesn't the problem still exist even with this patch because
>>> gss_add_msg() adds the msg onto the in_downcall() list? So gssd in
>>> __gss_fin_upcall() can find the 2nd upcall even before the 2nd msg is
>>> added to the pipe->pipe()?
>>
>> The use-after-free problem is solved I think. It doesn't really make
>> any difference if the down-call arrives before or after
>> rpc_queue_upcall() is called. The msg will still not be freed before it
>> is removed from both lists.
>>
>
> Sorry I don't see it.
Maybe we are looking at different code?
>
> Thread 1 adds an upcall and it's getting processed by gssd.
> Thread 2 executes gss_add_msg() which puts the message on the
> in_downcall list. Context switch (before the atomic_inc()!).
gss_add_msg(), as of 4.9-rc8, is
spin_lock(&pipe->lock);
old = __gss_find_upcall(pipe, gss_msg->uid, gss_msg->auth);
if (old == NULL) {
atomic_inc(&gss_msg->count);
list_add(&gss_msg->list, &pipe->in_downcall);
} else
gss_msg = old;
spin_unlock(&pipe->lock);
so the gss_msg is added to in_downcall *after* the atomic_inc(), and the
whole is protected by pipe->lock anyway so even if the atomic_inc() were
delayed by the CPU reordering things, there would be no risk of
gss_pipe_downcall() finding a gss_msg which didn't have the ->count
elevated.
NeilBrown
> Upcall comes back from the gssd, finds msg from Thread2 in_downcall
> list. gss_release_msg() will dec the counter to 0 and will remove the
> msg.
Attachment:
signature.asc
Description: PGP signature
