Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > Also isn't the reason why slub exists so it can track memory regions > per-cpu. call_rcu() and kfree_rcu() will add a grace period (multiple ms) where the cpu will likely evict from its caches the data contained in the 'about to be

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 18:09, Eric Dumazet wrote: > On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > > > My rationale was like this: we already have rcu to free the wq, so we > > don't add any more callbacks as current code. sock_alloc is right now > > 1136 bytes, which is huge,

Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > My rationale was like this: we already have rcu to free the wq, so we > don't add any more callbacks as current code. sock_alloc is right now > 1136 bytes, which is huge, like 18 cachelines. I wouldn't think it does > matter a lot

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 16:51, Eric Dumazet wrote: > On Thu, 2015-11-26 at 14:32 +0100, Hannes Frederic Sowa wrote: > > Hannes Frederic Sowa writes: > > > > > > > I have seen filesystems already doing so in .destroy_inode, that's why I > > > am asking. The allocation happens the same way as we

Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 14:32 +0100, Hannes Frederic Sowa wrote: > Hannes Frederic Sowa writes: > > > > I have seen filesystems already doing so in .destroy_inode, that's why I > > am asking. The allocation happens the same way as we do with sock_alloc, > > e.g. shmem. I actually thought that

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 14:32, Hannes Frederic Sowa wrote: > diff --git a/include/net/sock.h b/include/net/sock.h > index 7f89e4b..ae34da1 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1674,7 +1674,7 @@ static inline void sock_orphan(struct sock *sk) > static inline void

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
Hannes Frederic Sowa writes: > I have seen filesystems already doing so in .destroy_inode, that's why I > am asking. The allocation happens the same way as we do with sock_alloc, > e.g. shmem. I actually thought that struct inode already provides an > rcu_head for exactly that reason. E.g.:

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
Hannes Frederic Sowa writes: > I have seen filesystems already doing so in .destroy_inode, that's why I > am asking. The allocation happens the same way as we do with sock_alloc, > e.g. shmem. I actually thought that struct inode already provides an > rcu_head for

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 14:32, Hannes Frederic Sowa wrote: > diff --git a/include/net/sock.h b/include/net/sock.h > index 7f89e4b..ae34da1 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1674,7 +1674,7 @@ static inline void sock_orphan(struct sock *sk) > static inline void

Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 14:32 +0100, Hannes Frederic Sowa wrote: > Hannes Frederic Sowa writes: > > > > I have seen filesystems already doing so in .destroy_inode, that's why I > > am asking. The allocation happens the same way as we do with sock_alloc, > > e.g. shmem.

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 16:51, Eric Dumazet wrote: > On Thu, 2015-11-26 at 14:32 +0100, Hannes Frederic Sowa wrote: > > Hannes Frederic Sowa writes: > > > > > > > I have seen filesystems already doing so in .destroy_inode, that's why I > > > am asking. The

Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > My rationale was like this: we already have rcu to free the wq, so we > don't add any more callbacks as current code. sock_alloc is right now > 1136 bytes, which is huge, like 18 cachelines. I wouldn't think it does > matter a lot

Re: use-after-free in sock_wake_async

2015-11-26 Thread Hannes Frederic Sowa
On Thu, Nov 26, 2015, at 18:09, Eric Dumazet wrote: > On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > > > My rationale was like this: we already have rcu to free the wq, so we > > don't add any more callbacks as current code. sock_alloc is right now > > 1136 bytes, which is huge,

Re: use-after-free in sock_wake_async

2015-11-26 Thread Eric Dumazet
On Thu, 2015-11-26 at 18:03 +0100, Hannes Frederic Sowa wrote: > Also isn't the reason why slub exists so it can track memory regions > per-cpu. call_rcu() and kfree_rcu() will add a grace period (multiple ms) where the cpu will likely evict from its caches the data contained in the 'about to be

Re: use-after-free in sock_wake_async

2015-11-25 Thread Hannes Frederic Sowa
On Wed, Nov 25, 2015, at 23:43, Eric Dumazet wrote: > On Wed, 2015-11-25 at 23:32 +0100, Hannes Frederic Sowa wrote: > > On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > > > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > > > > > I do agree that keeping the ->sk_data_ready

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 23:32 +0100, Hannes Frederic Sowa wrote: > On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > > > I do agree that keeping the ->sk_data_ready outside of the lock will > > > very likely have performance

Re: use-after-free in sock_wake_async

2015-11-25 Thread Hannes Frederic Sowa
On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > I do agree that keeping the ->sk_data_ready outside of the lock will > > very likely have performance advantages. That's just something I > > wouldn't have undertaken because I'd

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > I do agree that keeping the ->sk_data_ready outside of the lock will > very likely have performance advantages. That's just something I > wouldn't have undertaken because I'd be reluctant to make a fairly > complicated change to a lot of

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 11:50 -0800, Eric Dumazet wrote: > >> > other->sk_data_ready(other); >> > + unix_state_unlock(other); > > > Also, problem with such construct is that we wakeup a thread that will > block on the lock we hold. > > Beauty of

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 11:50 -0800, Eric Dumazet wrote: > > other->sk_data_ready(other); > > + unix_state_unlock(other); Also, problem with such construct is that we wakeup a thread that will block on the lock we hold. Beauty of sk_data_ready() is to call it once

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 19:38 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: > >> Eric Dumazet writes: > >> > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > >> > > >> >> In case this is wrong, it obviously implies

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: >> Eric Dumazet writes: >> > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: >> > >> >> In case this is wrong, it obviously implies that sk_sleep(sk) must not >> >> be used anywhere as it accesses the same

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > > > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not > >> be used anywhere as it accesses the same struck sock, hence, when

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not >> be used anywhere as it accesses the same struck sock, hence, when that >> can "suddenly" disappear despite locks are used in the way

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > In case this is wrong, it obviously implies that sk_sleep(sk) must not > be used anywhere as it accesses the same struck sock, hence, when that > can "suddenly" disappear despite locks are used in the way indicated > above, there is now

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 16:43 +, Rainer Weikusat wrote: >> Eric Dumazet writes: >> > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat >> > wrote: >> >> [...] >> >> >> It's also easy to verify: Swap the unix_state_lock and >> >> other->sk_data_ready and see if the

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 16:43 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat > > wrote: > > [...] > > >> It's also easy to verify: Swap the unix_state_lock and > >> other->sk_data_ready and see if the issue still occurs. Right now (this

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat > wrote: [...] >> It's also easy to verify: Swap the unix_state_lock and >> other->sk_data_ready and see if the issue still occurs. Right now (this >> may change after I had some sleep as it's pretty late for me), I don't

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
t->flags) clear_bit(SOCK_ASYNC_WAITDATA, >sk_socket->flags); and similar calls. [ 7040.574519] ============== [ 7040.581838] AddressSanitizer: heap-use-after-free in sock_wake_async [ 7040.588229] Read of size 8 by thread T977831: [ 70

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat > wrote: [...] >> It's also easy to verify: Swap the unix_state_lock and >> other->sk_data_ready and see if the issue still occurs. Right now (this >> may change

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 16:43 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat > > wrote: > > [...] > > >> It's also easy to verify: Swap the unix_state_lock and > >>

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 16:43 +, Rainer Weikusat wrote: >> Eric Dumazet writes: >> > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat >> > wrote: >> >> [...] >> >> >> It's also easy to

Re: use-after-free in sock_wake_async

2015-11-25 Thread Hannes Frederic Sowa
On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > I do agree that keeping the ->sk_data_ready outside of the lock will > > very likely have performance advantages. That's just something I > > wouldn't have undertaken because I'd

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 23:32 +0100, Hannes Frederic Sowa wrote: > On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > > > I do agree that keeping the ->sk_data_ready outside of the lock will > > > very likely have performance

Re: use-after-free in sock_wake_async

2015-11-25 Thread Hannes Frederic Sowa
On Wed, Nov 25, 2015, at 23:43, Eric Dumazet wrote: > On Wed, 2015-11-25 at 23:32 +0100, Hannes Frederic Sowa wrote: > > On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > > > On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > > > > > > > I do agree that keeping the ->sk_data_ready

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not >> be used anywhere as it accesses the same struck sock, hence, when that >> can "suddenly" disappear despite locks

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: >> Eric Dumazet writes: >> > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: >> > >> >> In case this is wrong, it obviously implies that sk_sleep(sk) must

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 19:38 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: > >> Eric Dumazet writes: > >> > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > >> > > >> >>

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 18:24 +, Rainer Weikusat wrote: > Eric Dumazet writes: > > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > > > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not > >> be used anywhere as it accesses the same

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: > In case this is wrong, it obviously implies that sk_sleep(sk) must not > be used anywhere as it accesses the same struck sock, hence, when that > can "suddenly" disappear despite locks are used in the way indicated > above, there is now

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 20:57 +, Rainer Weikusat wrote: > I do agree that keeping the ->sk_data_ready outside of the lock will > very likely have performance advantages. That's just something I > wouldn't have undertaken because I'd be reluctant to make a fairly > complicated change to a lot of

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
On Wed, 2015-11-25 at 11:50 -0800, Eric Dumazet wrote: > > other->sk_data_ready(other); > > + unix_state_unlock(other); Also, problem with such construct is that we wakeup a thread that will block on the lock we hold. Beauty of sk_data_ready() is to call it once

Re: use-after-free in sock_wake_async

2015-11-25 Thread Rainer Weikusat
Eric Dumazet writes: > On Wed, 2015-11-25 at 11:50 -0800, Eric Dumazet wrote: > >> > other->sk_data_ready(other); >> > + unix_state_unlock(other); > > > Also, problem with such construct is that we wakeup a thread that will > block on the lock

Re: use-after-free in sock_wake_async

2015-11-25 Thread Eric Dumazet
t->flags) clear_bit(SOCK_ASYNC_WAITDATA, >sk_socket->flags); and similar calls. [ 7040.574519] ============== [ 7040.581838] AddressSanitizer: heap-use-after-free in sock_wake_async [ 7040.588229] Read of size 8 by thread T977831: [ 70

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, 2015-11-24 at 18:28 -0800, Eric Dumazet wrote: > Dmitry, could you test following patch with your setup ? > > ( I tried to reproduce the error you reported but could not ) > > Inode can be freed (without RCU grace period), but not the socket or > sk_wq > > By using sk_wq in the critical

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
Dmitry, could you test following patch with your setup ? ( I tried to reproduce the error you reported but could not ) Inode can be freed (without RCU grace period), but not the socket or sk_wq By using sk_wq in the critical paths, we do not dereference the inode, Thanks !

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat wrote: > > The af_unix part of this, yes, ie, what gets allocated in > unix_create1. But neither the socket inode nor the struct sock > originally passed to unix_create. Since these are part of the same > umbrella structure, they'll both be freed

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
Eric Dumazet writes: > On Tue, Nov 24, 2015 at 3:34 PM, Rainer Weikusat > wrote: >> Eric Dumazet writes: >>> On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: >>>> Hello, >>>> >>>> The following program triggers use-after-free in

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
Rainer Weikusat writes: [...] > Swap the unix_state_lock and s/lock/unlock/ :-( -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 3:34 PM, Rainer Weikusat wrote: > Eric Dumazet writes: >> On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: >>> Hello, >>> >>> The following program triggers use-after-free in sock_wake_async: > > [...] > >>> voi

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
Eric Dumazet writes: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: >> Hello, >> >> The following program triggers use-after-free in sock_wake_async: [...] >> void *thr1(void *arg) >> { >> syscall(SYS_close, r2, 0, 0, 0, 0, 0); >>

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 2:03 PM, Eric Dumazet wrote: > > This might be a data race in sk_wake_async() if inlined by compiler > (see https://lkml.org/lkml/2015/11/24/680 for another example) > > KASAN adds register pressure and compiler can then do 'stupid' things :( > > diff --git

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 1:45 PM, Benjamin LaHaise wrote: > On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: >> So looking at this trace I think its the other->sk_socket that gets >> freed and then we call sk_wake_async() on it. >> >> We could I think grab the socket reference there

Re: use-after-free in sock_wake_async

2015-11-24 Thread Benjamin LaHaise
On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: > So looking at this trace I think its the other->sk_socket that gets > freed and then we call sk_wake_async() on it. > > We could I think grab the socket reference there with unix_state_lock(), > since that is held by

Re: use-after-free in sock_wake_async

2015-11-24 Thread Al Viro
On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: > So looking at this trace I think its the other->sk_socket that gets > freed and then we call sk_wake_async() on it. > > We could I think grab the socket reference there with unix_state_lock(), > since that is held by

Re: use-after-free in sock_wake_async

2015-11-24 Thread Jason Baron
On 11/24/2015 10:21 AM, Eric Dumazet wrote: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: >> Hello, >> >> The following program triggers use-after-free in sock_wake_async: >> >> // autogenerated by syzkaller (http://github.com/google/syzkaller) >

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, 2015-11-24 at 07:21 -0800, Eric Dumazet wrote: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: > > Hello, > > > > The following program triggers use-after-free in sock_wake_async: > > > > // autogenerated by syzkaller (http://github.com/google/syzk

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: > Hello, > > The following program triggers use-after-free in sock_wake_async: > > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include > #include > #include > #include > > long r

use-after-free in sock_wake_async

2015-11-24 Thread Dmitry Vyukov
Hello, The following program triggers use-after-free in sock_wake_async: // autogenerated by syzkaller (http://github.com/google/syzkaller) #include #include #include #include long r2 = -1; long r3 = -1; long r7 = -1; void *thr0(void *arg) { syscall(SYS_splice, r2, 0x0ul, r7, 0x0ul

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
Eric Dumazet <eduma...@google.com> writes: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov <dvyu...@google.com> wrote: >> Hello, >> >> The following program triggers use-after-free in sock_wake_async: [...] >> void *thr1(void *arg) >> { &g

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 3:34 PM, Rainer Weikusat <rweiku...@mobileactivedefense.com> wrote: > Eric Dumazet <eduma...@google.com> writes: >> On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov <dvyu...@google.com> wrote: >>> Hello, >>> >>> The follow

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 2:03 PM, Eric Dumazet wrote: > > This might be a data race in sk_wake_async() if inlined by compiler > (see https://lkml.org/lkml/2015/11/24/680 for another example) > > KASAN adds register pressure and compiler can then do 'stupid' things :( > > diff

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 1:45 PM, Benjamin LaHaise wrote: > On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: >> So looking at this trace I think its the other->sk_socket that gets >> freed and then we call sk_wake_async() on it. >> >> We could I think grab the socket

Re: use-after-free in sock_wake_async

2015-11-24 Thread Benjamin LaHaise
On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: > So looking at this trace I think its the other->sk_socket that gets > freed and then we call sk_wake_async() on it. > > We could I think grab the socket reference there with unix_state_lock(), > since that is held by

Re: use-after-free in sock_wake_async

2015-11-24 Thread Jason Baron
On 11/24/2015 10:21 AM, Eric Dumazet wrote: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov <dvyu...@google.com> wrote: >> Hello, >> >> The following program triggers use-after-free in sock_wake_async: >> >> // autogenerated by syzkaller (http://g

Re: use-after-free in sock_wake_async

2015-11-24 Thread Al Viro
On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote: > So looking at this trace I think its the other->sk_socket that gets > freed and then we call sk_wake_async() on it. > > We could I think grab the socket reference there with unix_state_lock(), > since that is held by

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
Rainer Weikusat writes: [...] > Swap the unix_state_lock and s/lock/unlock/ :-( -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at

Re: use-after-free in sock_wake_async

2015-11-24 Thread Rainer Weikusat
<dvyu...@google.com> wrote: >>>> Hello, >>>> >>>> The following program triggers use-after-free in sock_wake_async: >> >> [...] >> >>>> void *thr1(void *arg) >>>> { >>>> syscall(SYS_close, r2, 0, 0, 0, 0, 0);

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat wrote: > > The af_unix part of this, yes, ie, what gets allocated in > unix_create1. But neither the socket inode nor the struct sock > originally passed to unix_create. Since these are part of the same > umbrella

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
Dmitry, could you test following patch with your setup ? ( I tried to reproduce the error you reported but could not ) Inode can be freed (without RCU grace period), but not the socket or sk_wq By using sk_wq in the critical paths, we do not dereference the inode, Thanks !

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, 2015-11-24 at 18:28 -0800, Eric Dumazet wrote: > Dmitry, could you test following patch with your setup ? > > ( I tried to reproduce the error you reported but could not ) > > Inode can be freed (without RCU grace period), but not the socket or > sk_wq > > By using sk_wq in the critical

use-after-free in sock_wake_async

2015-11-24 Thread Dmitry Vyukov
Hello, The following program triggers use-after-free in sock_wake_async: // autogenerated by syzkaller (http://github.com/google/syzkaller) #include #include #include #include long r2 = -1; long r3 = -1; long r7 = -1; void *thr0(void *arg) { syscall(SYS_splice, r2, 0x0ul, r7, 0x0ul

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov <dvyu...@google.com> wrote: > Hello, > > The following program triggers use-after-free in sock_wake_async: > > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include > #include > #include > #

Re: use-after-free in sock_wake_async

2015-11-24 Thread Eric Dumazet
On Tue, 2015-11-24 at 07:21 -0800, Eric Dumazet wrote: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov <dvyu...@google.com> wrote: > > Hello, > > > > The following program triggers use-after-free in sock_wake_async: > > > > // autogenerated by syzkal