Re: Use-after-free in ppoll

2015-11-23 Thread Dmitry Vyukov
On Sun, Nov 22, 2015 at 7:46 PM, Rainer Weikusat
 wrote:
> Dmitry Vyukov  writes:
>> On Sun, Nov 22, 2015 at 3:32 PM, Rainer Weikusat
>>  wrote:
>>> Dmitry Vyukov  writes:
 Hello,

 On commit f2d10565b9bdbb722bd43e6e1a759eeddb9645c8 (Nov 20).

 The following program triggers use-after-free:

 // autogenerated by syzkaller (http://github.com/google/syzkaller)
 #include 
 #include 
 #include 
 #include 

 void *thread(void *p)
 {
 syscall(SYS_write, (long)p, 0x2000278ful, 0x1ul, 0, 0, 0);
 return 0;
 }
>>>
>>> [...]
>>>
>>>
 long r1 = syscall(SYS_socketpair, 0x1ul, 0x3ul, 0x0ul,
>>>
>>> [...]
>>>
 long r5 = syscall(SYS_close, r2, 0, 0, 0, 0, 0);
 pthread_t th;
 pthread_create(, 0, thread, (void*)(long)r3);
>>>
>>> [...]
>>>
 long r21 = syscall(SYS_ppoll, 0x2ffful, 0x3ul, 0x2ffcul, 
 0x2ffdul, 0x8ul, 0);
 return 0;
 }
>>>
>>> That's one of the already known sequences for triggering this issue:
>
> [...]
>
>> I have not read the code. But I just want to point out that all 3
>> reports are different. For example, in the first one, ppoll both frees
>> the object and then accesses it. That is, it is not write that frees
>> the object.
>
> The call trace is always the same:
>
> [ 2672.994366]  [] __asan_load4+0x6a/0x70
> [ 2672.994366]  [] do_raw_spin_lock+0x22/0x220
> [ 2672.994366]  [] _raw_spin_lock_irqsave+0x51/0x60
> [ 2672.994366]  [] remove_wait_queue+0x18/0x80
> [ 2672.994366]  [] poll_freewait+0x7b/0x130
> [ 2672.994366]  [] do_sys_poll+0x4dc/0x860
> [ 2672.994366]  [] SyS_ppoll+0x1a9/0x310
>
> And if you look at the poll implementation, the important part is this
> (fs/ select.c, do_sys_poll)
>
> fdcount = do_poll(nfds, head, , end_time);
> poll_freewait();
>
> do_poll calls the poll routine of the file descriptors which cause
> "enqueuing of something" via poll wait callback. For poll, that's the
> __pollwait routine in select.c:
>
> static void __pollwait(struct file *filp, wait_queue_head_t *wait_address,
> poll_table *p)
> {
> struct poll_wqueues *pwq = container_of(p, struct poll_wqueues, pt);
> struct poll_table_entry *entry = poll_get_entry(pwq);
> if (!entry)
> return;
> entry->filp = get_file(filp);
> entry->wait_address = wait_address;
> entry->key = p->_key;
> init_waitqueue_func_entry(>wait, pollwake);
> entry->wait.private = pwq;
> add_wait_queue(wait_address, >wait);
> }
>
> because of the close, this routine will be called with the peer_wait
> wait_queue_head of the non-closed socket of the socket pair as
> wait_address argument. And poll_freewait calls free_poll_entry for all
> entries on the poll table which is
>
> static void free_poll_entry(struct poll_table_entry *entry)
> {
> remove_wait_queue(entry->wait_address, >wait);
> fput(entry->filp);
> }
>
> but by this time, the wait_address points to freed memory because the
> only thing which kept the socket it belonged to alive after the
> corresponding file descriptor was closed was the reference the other
> socket held. But that was dropped by unix_dgram_sendmsg upon detecting a
> dead peer.


Hi Rainer,

I am not questioning your conclusions. You definitely know better.

Btw, how close are you to a fix that everybody is happy with? I hit
this use-after-free very frequently.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Use-after-free in ppoll

2015-11-22 Thread Dmitry Vyukov
On Sun, Nov 22, 2015 at 3:32 PM, Rainer Weikusat
 wrote:
> Dmitry Vyukov  writes:
>> Hello,
>>
>> On commit f2d10565b9bdbb722bd43e6e1a759eeddb9645c8 (Nov 20).
>>
>> The following program triggers use-after-free:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> void *thread(void *p)
>> {
>> syscall(SYS_write, (long)p, 0x2000278ful, 0x1ul, 0, 0, 0);
>> return 0;
>> }
>
> [...]
>
>
>> long r1 = syscall(SYS_socketpair, 0x1ul, 0x3ul, 0x0ul,
>
> [...]
>
>> long r5 = syscall(SYS_close, r2, 0, 0, 0, 0, 0);
>> pthread_t th;
>> pthread_create(, 0, thread, (void*)(long)r3);
>
> [...]
>
>> long r21 = syscall(SYS_ppoll, 0x2ffful, 0x3ul, 0x2ffcul, 
>> 0x2ffdul, 0x8ul, 0);
>> return 0;
>> }
>
> That's one of the already known sequences for triggering this issue: The
> close will clear the peer pointer of the closed socket, hence, the 2nd
> sock_poll_wait will be called by unix_dgram_poll. The write will
> execute unix_dgram_sendmsg which detects that the peer is dead and
> disconnects from it, causing the corresponding structures to be freed
> despite they're still used.
>
> NB: I didn't execute this but I spend a fair amount of time with the
> af_unix.c code during the last couple of weeks and consider myself
> "reasonably familiar" with it and that's IMO what should happen here.


I have not read the code. But I just want to point out that all 3
reports are different. For example, in the first one, ppoll both frees
the object and then accesses it. That is, it is not write that frees
the object.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Use-after-free in ppoll

2015-11-22 Thread Rainer Weikusat
Dmitry Vyukov  writes:
> Hello,
>
> On commit f2d10565b9bdbb722bd43e6e1a759eeddb9645c8 (Nov 20).
>
> The following program triggers use-after-free:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include 
> #include 
> #include 
> #include 
>
> void *thread(void *p)
> {
> syscall(SYS_write, (long)p, 0x2000278ful, 0x1ul, 0, 0, 0);
> return 0;
> }

[...]


> long r1 = syscall(SYS_socketpair, 0x1ul, 0x3ul, 0x0ul,

[...]

> long r5 = syscall(SYS_close, r2, 0, 0, 0, 0, 0);
> pthread_t th;
> pthread_create(, 0, thread, (void*)(long)r3);

[...]

> long r21 = syscall(SYS_ppoll, 0x2ffful, 0x3ul, 0x2ffcul, 
> 0x2ffdul, 0x8ul, 0);
> return 0;
> }

That's one of the already known sequences for triggering this issue: The
close will clear the peer pointer of the closed socket, hence, the 2nd
sock_poll_wait will be called by unix_dgram_poll. The write will
execute unix_dgram_sendmsg which detects that the peer is dead and
disconnects from it, causing the corresponding structures to be freed
despite they're still used.

NB: I didn't execute this but I spend a fair amount of time with the
af_unix.c code during the last couple of weeks and consider myself
"reasonably familiar" with it and that's IMO what should happen here.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Use-after-free in ppoll

2015-11-22 Thread Rainer Weikusat
Dmitry Vyukov  writes:
> On Sun, Nov 22, 2015 at 3:32 PM, Rainer Weikusat
>  wrote:
>> Dmitry Vyukov  writes:
>>> Hello,
>>>
>>> On commit f2d10565b9bdbb722bd43e6e1a759eeddb9645c8 (Nov 20).
>>>
>>> The following program triggers use-after-free:
>>>
>>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>>> #include 
>>> #include 
>>> #include 
>>> #include 
>>>
>>> void *thread(void *p)
>>> {
>>> syscall(SYS_write, (long)p, 0x2000278ful, 0x1ul, 0, 0, 0);
>>> return 0;
>>> }
>>
>> [...]
>>
>>
>>> long r1 = syscall(SYS_socketpair, 0x1ul, 0x3ul, 0x0ul,
>>
>> [...]
>>
>>> long r5 = syscall(SYS_close, r2, 0, 0, 0, 0, 0);
>>> pthread_t th;
>>> pthread_create(, 0, thread, (void*)(long)r3);
>>
>> [...]
>>
>>> long r21 = syscall(SYS_ppoll, 0x2ffful, 0x3ul, 0x2ffcul, 
>>> 0x2ffdul, 0x8ul, 0);
>>> return 0;
>>> }
>>
>> That's one of the already known sequences for triggering this issue:

[...]

> I have not read the code. But I just want to point out that all 3
> reports are different. For example, in the first one, ppoll both frees
> the object and then accesses it. That is, it is not write that frees
> the object.

The call trace is always the same:

[ 2672.994366]  [] __asan_load4+0x6a/0x70
[ 2672.994366]  [] do_raw_spin_lock+0x22/0x220
[ 2672.994366]  [] _raw_spin_lock_irqsave+0x51/0x60
[ 2672.994366]  [] remove_wait_queue+0x18/0x80
[ 2672.994366]  [] poll_freewait+0x7b/0x130
[ 2672.994366]  [] do_sys_poll+0x4dc/0x860
[ 2672.994366]  [] SyS_ppoll+0x1a9/0x310

And if you look at the poll implementation, the important part is this
(fs/ select.c, do_sys_poll)

fdcount = do_poll(nfds, head, , end_time);
poll_freewait();

do_poll calls the poll routine of the file descriptors which cause
"enqueuing of something" via poll wait callback. For poll, that's the
__pollwait routine in select.c:

static void __pollwait(struct file *filp, wait_queue_head_t *wait_address,
poll_table *p)
{
struct poll_wqueues *pwq = container_of(p, struct poll_wqueues, pt);
struct poll_table_entry *entry = poll_get_entry(pwq);
if (!entry)
return;
entry->filp = get_file(filp);
entry->wait_address = wait_address;
entry->key = p->_key;
init_waitqueue_func_entry(>wait, pollwake);
entry->wait.private = pwq;
add_wait_queue(wait_address, >wait);
}

because of the close, this routine will be called with the peer_wait
wait_queue_head of the non-closed socket of the socket pair as
wait_address argument. And poll_freewait calls free_poll_entry for all
entries on the poll table which is

static void free_poll_entry(struct poll_table_entry *entry)
{
remove_wait_queue(entry->wait_address, >wait);
fput(entry->filp);
}

but by this time, the wait_address points to freed memory because the
only thing which kept the socket it belonged to alive after the
corresponding file descriptor was closed was the reference the other
socket held. But that was dropped by unix_dgram_sendmsg upon detecting a
dead peer.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Use-after-free in ppoll

2015-11-22 Thread Rainer Weikusat
Rainer Weikusat  writes:

[...]


> because of the close, this routine will be called with the peer_wait
> wait_queue_head of the non-closed socket of the socket pair as
> wait_address argument.

This should have been "peer_wait wait_queue_head of the peer of the
non-closed socket, ie, that of the closed socket"...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html