[Lguest] [PATCH 4/5] lguest: use KVM hypercalls

[Patrick McHardy]

Eric W. Biederman wrote:
> Patrick McHardy <kaber at trash.net> writes:
> 
>> When creating the device using tunctl the sk->sk_sleep poiner is
>> set to the read_wait completion of the file opened by tunctl, but
>> it is not refreshed when attaching to lguest or released when
>> closing the file, causing a stale pointer dereference in
>> tun_sock_write_space().
>>
>> Eric, please review. Thanks.
> 
> That looks a little better.  Certainly as the socket currently
> lives with the tun_struct instead of the tun_file it make sense.
> I'm not at all certain it makes sense for the socket to live in
> tun_struct instead of tun_file.
> 
> I happened to glance at the code about a week ago, and realized that
> the introduction of the socket had done horribly things to the
> guarantees I had introduced, and I haven't had a chance to think
> through and figure out what the code should be doing.
> 
> I am certain that:
> opening a tap device and then "ip link del tap0" while holding
> the tap open leads into a territory of madness right now.
> 
> And apparently so does reattaching to an existing tun device.
> 
> Patrick I'm not seeing anything in the particular patch you pointed
> out that would cause crashes.

It might have been a different patch or a combination, I assumed it
was your patch since git annotate pointed to it and it was a very
recent change.

> Other lurking bugs aside your patch appears slightly off.
> 
> tun->sk->sk_sleep in __tun_detach is correct.
> 
> Setting sk_sleep on both paths to tun_attach instead
> of in tun_attach is wrong.  You are performing the assignment
> before we complete the permission checks into tun_attach, which
> means there is no guarantee that the tun_attach will succeed.

I see. How about this patch instead? It moves the sk_sleep assignment
to tun_attach, after the permission checks took place.

Thanks.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: x
URL: <http://lists.ozlabs.org/pipermail/lguest/attachments/20090414/341b30a5/attachment.asc>