Re: [PATCH] tun: Fix use-after-free in tun_net_xmit

[Jason Wang]

On 2019/4/29 äå1:59, Cong Wang wrote:
> On Sun, Apr 28, 2019 at 12:51 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
>>> tun_net_xmit() doesn't have the chance to
>>> access the change because it holding the rcu_read_lock().
>>
>>
>> The problem is the following codes:
>>
>>
>>          --tun->numqueues;
>>
>>          ...
>>
>>          synchronize_net();
>>
>> We need make sure the decrement of tun->numqueues be visible to readers
>> after synchronize_net(). And in tun_net_xmit():
>
> It doesn't matter at all. Readers are okay to read it even they still use the
> stale tun->numqueues, as long as the tfile is not freed readers can read
> whatever they want...

This is only true if we set SOCK_RCU_FREE, isn't it?

>
> The decrement of tun->numqueues is just how we unpublish the old
> tfile, it is still valid for readers to read it _after_ unpublish, we only need
> to worry about free, not about unpublish. This is the whole spirit of RCU.
>

The point is we don't convert tun->numqueues to RCU but use
synchronize_net().

> You need to rethink about my SOCK_RCU_FREE patch.

The code is wrote before SOCK_RCU_FREE is introduced and assume no
de-reference from device after synchronize_net(). It doesn't harm to
figure out the root cause which may give us more confidence to the fix
(e.g like SOCK_RCU_FREE).

I don't object to fix with SOCK_RCU_FREE, but then we should remove
the redundant synchronize_net(). But I still prefer to synchronize
everything explicitly like (completely untested):