From: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp
commit 6f7c41374b62fd80bbd8aae3536c43688c54d95e upstream.
syzbot is reporting that use of SOCKET_I()->sk from open() can result in use after free problem [1], for socket's inode is still reachable via /proc/pid/fd/n despite destruction of SOCKET_I()->sk already completed.
At first I thought that this race condition applies to only open/getattr permission checks. But James Morris has pointed out that there are more permission checks where this race condition applies to. Thus, get rid of tomoyo_get_socket_name() instead of conditionally bypassing permission checks on sockets. As a side effect of this patch, "socket:[family=$:type=$:protocol=$]" in the policy files has to be rewritten to "socket:[$]".
[1] https://syzkaller.appspot.com/bug?id=73d590010454403d55164cca23bd0565b1eb3b7...
Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp Reported-by: syzbot syzbot+0341f6a4d729d4e0acf1@syzkaller.appspotmail.com Reported-by: James Morris jmorris@namei.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- security/tomoyo/realpath.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-)
--- a/security/tomoyo/realpath.c +++ b/security/tomoyo/realpath.c @@ -218,31 +218,6 @@ out: }
/** - * tomoyo_get_socket_name - Get the name of a socket. - * - * @path: Pointer to "struct path". - * @buffer: Pointer to buffer to return value in. - * @buflen: Sizeof @buffer. - * - * Returns the buffer. - */ -static char *tomoyo_get_socket_name(const struct path *path, char * const buffer, - const int buflen) -{ - struct inode *inode = d_backing_inode(path->dentry); - struct socket *sock = inode ? SOCKET_I(inode) : NULL; - struct sock *sk = sock ? sock->sk : NULL; - - if (sk) { - snprintf(buffer, buflen, "socket:[family=%u:type=%u:protocol=%u]", - sk->sk_family, sk->sk_type, sk->sk_protocol); - } else { - snprintf(buffer, buflen, "socket:[unknown]"); - } - return buffer; -} - -/** * tomoyo_realpath_from_path - Returns realpath(3) of the given pathname but ignores chroot'ed root. * * @path: Pointer to "struct path". @@ -279,12 +254,7 @@ char *tomoyo_realpath_from_path(const st break; /* To make sure that pos is '\0' terminated. */ buf[buf_len - 1] = '\0'; - /* Get better name for socket. */ - if (sb->s_magic == SOCKFS_MAGIC) { - pos = tomoyo_get_socket_name(path, buf, buf_len - 1); - goto encode; - } - /* For "pipe:[$]". */ + /* For "pipe:[$]" and "socket:[$]". */ if (dentry->d_op && dentry->d_op->d_dname) { pos = dentry->d_op->d_dname(dentry, buf, buf_len - 1); goto encode;