tomoyo: Don't use nifty names on sockets. (6f7c4137) · Commits · 戴 / test

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=73d590010454403d55164cca23bd0565b1eb3b74 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>

security/tomoyo/realpath.c

+1 −31

Original line number	Diff line number	Diff line
		@@ -217,31 +217,6 @@ out:
		return ERR_PTR(-ENOMEM);
		}

		/**
		* 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.
		*
		@@ -279,12 +254,7 @@ char tomoyo_realpath_from_path(const struct path path)
		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;

Admin message