Commit 355f0746 authored by Marco Elver's avatar Marco Elver Committed by Dominique Martinet
Browse files

9p/trans_fd: Annotate data-racy writes to file::f_flags 



syzbot reported:

 | BUG: KCSAN: data-race in p9_fd_create / p9_fd_create
 |
 | read-write to 0xffff888130fb3d48 of 4 bytes by task 15599 on cpu 0:
 |  p9_fd_open net/9p/trans_fd.c:842 [inline]
 |  p9_fd_create+0x210/0x250 net/9p/trans_fd.c:1092
 |  p9_client_create+0x595/0xa70 net/9p/client.c:1010
 |  v9fs_session_init+0xf9/0xd90 fs/9p/v9fs.c:410
 |  v9fs_mount+0x69/0x630 fs/9p/vfs_super.c:123
 |  legacy_get_tree+0x74/0xd0 fs/fs_context.c:611
 |  vfs_get_tree+0x51/0x190 fs/super.c:1519
 |  do_new_mount+0x203/0x660 fs/namespace.c:3335
 |  path_mount+0x496/0xb30 fs/namespace.c:3662
 |  do_mount fs/namespace.c:3675 [inline]
 |  __do_sys_mount fs/namespace.c:3884 [inline]
 |  [...]
 |
 | read-write to 0xffff888130fb3d48 of 4 bytes by task 15563 on cpu 1:
 |  p9_fd_open net/9p/trans_fd.c:842 [inline]
 |  p9_fd_create+0x210/0x250 net/9p/trans_fd.c:1092
 |  p9_client_create+0x595/0xa70 net/9p/client.c:1010
 |  v9fs_session_init+0xf9/0xd90 fs/9p/v9fs.c:410
 |  v9fs_mount+0x69/0x630 fs/9p/vfs_super.c:123
 |  legacy_get_tree+0x74/0xd0 fs/fs_context.c:611
 |  vfs_get_tree+0x51/0x190 fs/super.c:1519
 |  do_new_mount+0x203/0x660 fs/namespace.c:3335
 |  path_mount+0x496/0xb30 fs/namespace.c:3662
 |  do_mount fs/namespace.c:3675 [inline]
 |  __do_sys_mount fs/namespace.c:3884 [inline]
 |  [...]
 |
 | value changed: 0x00008002 -> 0x00008802

Within p9_fd_open(), O_NONBLOCK is added to f_flags of the read and
write files. This may happen concurrently if e.g. mounting process
modifies the fd in another thread.

Mark the plain read-modify-writes as intentional data-races, with the
assumption that the result of executing the accesses concurrently will
always result in the same result despite the accesses themselves not
being atomic.

Reported-by: default avatar <syzbot+e441aeeb422763cc5511@syzkaller.appspotmail.com>
Signed-off-by: default avatarMarco Elver <elver@google.com>
Link: https://lore.kernel.org/r/ZO38mqkS0TYUlpFp@elver.google.com


Signed-off-by: default avatarDominique Martinet <asmadeus@codewreck.org>
Message-ID: <20231025103445.1248103-1-asmadeus@codewreck.org>
parent a321af9d

net/9p/trans_fd.c

+10 −3
Original line number Diff line number Diff line
@@ -836,14 +836,21 @@ static int p9_fd_open(struct p9_client *client, int rfd, int wfd) 
		goto out_free_ts;

	if (!(ts->rd->f_mode & FMODE_READ))

		goto out_put_rd;

	/* prevent workers from hanging on IO when fd is a pipe */

	ts->rd->f_flags |= O_NONBLOCK;

	/* Prevent workers from hanging on IO when fd is a pipe.

	 * It's technically possible for userspace or concurrent mounts to

	 * modify this flag concurrently, which will likely result in a

	 * broken filesystem. However, just having bad flags here should

	 * not crash the kernel or cause any other sort of bug, so mark this

	 * particular data race as intentional so that tooling (like KCSAN)

	 * can allow it and detect further problems.

	 */

	data_race(ts->rd->f_flags |= O_NONBLOCK);

	ts->wr = fget(wfd);

	if (!ts->wr)

		goto out_put_rd;

	if (!(ts->wr->f_mode & FMODE_WRITE))

		goto out_put_wr;

	ts->wr->f_flags |= O_NONBLOCK;

	data_race(ts->wr->f_flags |= O_NONBLOCK);



	client->trans = ts;

	client->status = Connected;