Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-10 Thread Cong Wang
On Sun, Jun 10, 2018 at 12:27 PM, David Miller wrote: > I'm applying this for now, it is at least a step towards fixing > all of these issues. > > If it is really offensive, I can revert, just tell me. Thanks, David! Unless there is something fundamentally broken, there is no reason to revert

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-10 Thread David Miller
From: Cong Wang Date: Thu, 7 Jun 2018 13:39:49 -0700 > fchownat() doesn't even hold refcnt of fd until it figures out > fd is really needed (otherwise is ignored) and releases it after > it resolves the path. This means sock_close() could race with > sockfs_setattr(), which leads to a NULL

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Al Viro
On Thu, Jun 07, 2018 at 03:15:15PM -0700, Cong Wang wrote: > > You do realize that the same ->setattr() can be called by way of > > chown() on /proc/self/fd/, right? What would you do there - > > bump refcount on that struct file when traversing that symlink and > > hold it past the end of

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Cong Wang
On Thu, Jun 7, 2018 at 3:04 PM, Al Viro wrote: > On Thu, Jun 07, 2018 at 02:45:58PM -0700, Cong Wang wrote: >> On Thu, Jun 7, 2018 at 2:26 PM, Al Viro wrote: >> > On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: >> >> fchownat() doesn't even hold refcnt of fd until it figures out >> >>

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Al Viro
On Thu, Jun 07, 2018 at 02:45:58PM -0700, Cong Wang wrote: > On Thu, Jun 7, 2018 at 2:26 PM, Al Viro wrote: > > On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: > >> fchownat() doesn't even hold refcnt of fd until it figures out > >> fd is really needed (otherwise is ignored) and

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Cong Wang
On Thu, Jun 7, 2018 at 2:26 PM, Al Viro wrote: > On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: >> fchownat() doesn't even hold refcnt of fd until it figures out >> fd is really needed (otherwise is ignored) and releases it after >> it resolves the path. This means sock_close() could

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Al Viro
On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: > fchownat() doesn't even hold refcnt of fd until it figures out > fd is really needed (otherwise is ignored) and releases it after > it resolves the path. This means sock_close() could race with > sockfs_setattr(), which leads to a NULL

[Patch net] socket: close race condition between sock_close() and sockfs_setattr()

2018-06-07 Thread Cong Wang
fchownat() doesn't even hold refcnt of fd until it figures out fd is really needed (otherwise is ignored) and releases it after it resolves the path. This means sock_close() could race with sockfs_setattr(), which leads to a NULL pointer dereference since typically we set sock->sk to NULL in