Skip to content

Commit 6f7c413

Browse files
Tetsuo HandaTetsuo Handa
authored andcommitted
tomoyo: Don't use nifty names on sockets.
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 <[email protected]> Reported-by: syzbot <[email protected]> Reported-by: James Morris <[email protected]>
1 parent 6794862 commit 6f7c413

File tree

1 file changed

+1
-31
lines changed

1 file changed

+1
-31
lines changed

security/tomoyo/realpath.c

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -217,31 +217,6 @@ static char *tomoyo_get_local_path(struct dentry *dentry, char * const buffer,
217217
return ERR_PTR(-ENOMEM);
218218
}
219219

220-
/**
221-
* tomoyo_get_socket_name - Get the name of a socket.
222-
*
223-
* @path: Pointer to "struct path".
224-
* @buffer: Pointer to buffer to return value in.
225-
* @buflen: Sizeof @buffer.
226-
*
227-
* Returns the buffer.
228-
*/
229-
static char *tomoyo_get_socket_name(const struct path *path, char * const buffer,
230-
const int buflen)
231-
{
232-
struct inode *inode = d_backing_inode(path->dentry);
233-
struct socket *sock = inode ? SOCKET_I(inode) : NULL;
234-
struct sock *sk = sock ? sock->sk : NULL;
235-
236-
if (sk) {
237-
snprintf(buffer, buflen, "socket:[family=%u:type=%u:protocol=%u]",
238-
sk->sk_family, sk->sk_type, sk->sk_protocol);
239-
} else {
240-
snprintf(buffer, buflen, "socket:[unknown]");
241-
}
242-
return buffer;
243-
}
244-
245220
/**
246221
* tomoyo_realpath_from_path - Returns realpath(3) of the given pathname but ignores chroot'ed root.
247222
*
@@ -279,12 +254,7 @@ char *tomoyo_realpath_from_path(const struct path *path)
279254
break;
280255
/* To make sure that pos is '\0' terminated. */
281256
buf[buf_len - 1] = '\0';
282-
/* Get better name for socket. */
283-
if (sb->s_magic == SOCKFS_MAGIC) {
284-
pos = tomoyo_get_socket_name(path, buf, buf_len - 1);
285-
goto encode;
286-
}
287-
/* For "pipe:[\$]". */
257+
/* For "pipe:[\$]" and "socket:[\$]". */
288258
if (dentry->d_op && dentry->d_op->d_dname) {
289259
pos = dentry->d_op->d_dname(dentry, buf, buf_len - 1);
290260
goto encode;

0 commit comments

Comments
 (0)