shinys000114/linux
1
0
Fork 0
mirror of https://github.com/hardkernel/linux.git synced 2026-06-10 04:48:04 +09:00
Code Issues Packages Projects Releases Wiki Activity

UPSTREAM: socket: close race condition between sock_close() and sockfs_setattr()

Browse Source 
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 ->release().

As pointed out by Al, this is unique to sockfs. So we can fix this
in socket layer by acquiring inode_lock in sock_close() and
checking against NULL in sockfs_setattr().

sock_release() is called in many places, only the sock_close()
path matters here. And fortunately, this should not affect normal
sock_close() as it is only called when the last fd refcnt is gone.
It only affects sock_close() with a parallel sockfs_setattr() in
progress, which is not common.

Fixes: 86741ec254 ("net: core: Add a UID field to struct sock.")
Reported-by: shankarapailoor <shankarapailoor@gmail.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

(cherry picked from commit 6d8c50dcb0)
Signed-off-by: Chenbo Feng <fengc@google.com>

Bug: 112220999
Test: syzcaller reproducer doesn't trigger the crash anymore
Change-Id: I90bec1515889e0dfd23f94e3f29b366c7bbfcd11
This commit is contained in:
Cong Wang
2018-06-07 13:39:49 -07:00
committed by Chenbo Feng
parent 37af2ff398
commit 3a2ec581f8
1 changed files with 15 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
net/socket.c
View File

@@ -528,7 +528,10 @@ static int sockfs_setattr(struct dentry *dentry, struct iattr *iattr)
if (!err && (iattr->ia_valid & ATTR_UID)) {
struct socket *sock = SOCKET_I(d_inode(dentry));
sock->sk->sk_uid = iattr->ia_uid;
if (sock->sk)
sock->sk->sk_uid = iattr->ia_uid;
else
err = -ENOENT;
}
return err;
@@ -579,12 +582,16 @@ static struct socket *sock_alloc(void)
* an inode not a file.
*/
void sock_release(struct socket *sock)
static void __sock_release(struct socket *sock, struct inode *inode)
{
if (sock->ops) {
struct module *owner = sock->ops->owner;
if (inode)
inode_lock(inode);
sock->ops->release(sock);
if (inode)
inode_unlock(inode);
sock->ops = NULL;
module_put(owner);
}
@@ -599,6 +606,11 @@ void sock_release(struct socket *sock)
}
sock->file = NULL;
}
void sock_release(struct socket *sock)
{
__sock_release(sock, NULL);
}
EXPORT_SYMBOL(sock_release);
void __sock_tx_timestamp(const struct sock *sk, __u8 *tx_flags)
@@ -1035,7 +1047,7 @@ static int sock_mmap(struct file *file, struct vm_area_struct *vma)
static int sock_close(struct inode *inode, struct file *filp)
{
sock_release(SOCKET_I(inode));
__sock_release(SOCKET_I(inode), inode);
return 0;
}