]>
Commit | Line | Data |
---|---|---|
335f7cc0 SL |
1 | From cc654bac7372327bd75399d7806b5ee7287f4b2e Mon Sep 17 00:00:00 2001 |
2 | From: Sasha Levin <sashal@kernel.org> | |
3 | Date: Mon, 10 Apr 2023 21:04:50 +0900 | |
4 | Subject: sysv: don't call sb_bread() with pointers_lock held | |
5 | ||
6 | From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> | |
7 | ||
8 | [ Upstream commit f123dc86388cb669c3d6322702dc441abc35c31e ] | |
9 | ||
10 | syzbot is reporting sleep in atomic context in SysV filesystem [1], for | |
11 | sb_bread() is called with rw_spinlock held. | |
12 | ||
13 | A "write_lock(&pointers_lock) => read_lock(&pointers_lock) deadlock" bug | |
14 | and a "sb_bread() with write_lock(&pointers_lock)" bug were introduced by | |
15 | "Replace BKL for chain locking with sysvfs-private rwlock" in Linux 2.5.12. | |
16 | ||
17 | Then, "[PATCH] err1-40: sysvfs locking fix" in Linux 2.6.8 fixed the | |
18 | former bug by moving pointers_lock lock to the callers, but instead | |
19 | introduced a "sb_bread() with read_lock(&pointers_lock)" bug (which made | |
20 | this problem easier to hit). | |
21 | ||
22 | Al Viro suggested that why not to do like get_branch()/get_block()/ | |
23 | find_shared() in Minix filesystem does. And doing like that is almost a | |
24 | revert of "[PATCH] err1-40: sysvfs locking fix" except that get_branch() | |
25 | from with find_shared() is called without write_lock(&pointers_lock). | |
26 | ||
27 | Reported-by: syzbot <syzbot+69b40dc5fd40f32c199f@syzkaller.appspotmail.com> | |
28 | Link: https://syzkaller.appspot.com/bug?extid=69b40dc5fd40f32c199f | |
29 | Suggested-by: Al Viro <viro@zeniv.linux.org.uk> | |
30 | Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> | |
31 | Link: https://lore.kernel.org/r/0d195f93-a22a-49a2-0020-103534d6f7f6@I-love.SAKURA.ne.jp | |
32 | Signed-off-by: Christian Brauner <brauner@kernel.org> | |
33 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
34 | --- | |
35 | fs/sysv/itree.c | 10 ++++------ | |
36 | 1 file changed, 4 insertions(+), 6 deletions(-) | |
37 | ||
38 | diff --git a/fs/sysv/itree.c b/fs/sysv/itree.c | |
39 | index 410ab2a44d2f6..19bcb51a22036 100644 | |
40 | --- a/fs/sysv/itree.c | |
41 | +++ b/fs/sysv/itree.c | |
42 | @@ -83,9 +83,6 @@ static inline sysv_zone_t *block_end(struct buffer_head *bh) | |
43 | return (sysv_zone_t*)((char*)bh->b_data + bh->b_size); | |
44 | } | |
45 | ||
46 | -/* | |
47 | - * Requires read_lock(&pointers_lock) or write_lock(&pointers_lock) | |
48 | - */ | |
49 | static Indirect *get_branch(struct inode *inode, | |
50 | int depth, | |
51 | int offsets[], | |
52 | @@ -105,15 +102,18 @@ static Indirect *get_branch(struct inode *inode, | |
53 | bh = sb_bread(sb, block); | |
54 | if (!bh) | |
55 | goto failure; | |
56 | + read_lock(&pointers_lock); | |
57 | if (!verify_chain(chain, p)) | |
58 | goto changed; | |
59 | add_chain(++p, bh, (sysv_zone_t*)bh->b_data + *++offsets); | |
60 | + read_unlock(&pointers_lock); | |
61 | if (!p->key) | |
62 | goto no_block; | |
63 | } | |
64 | return NULL; | |
65 | ||
66 | changed: | |
67 | + read_unlock(&pointers_lock); | |
68 | brelse(bh); | |
69 | *err = -EAGAIN; | |
70 | goto no_block; | |
71 | @@ -219,9 +219,7 @@ static int get_block(struct inode *inode, sector_t iblock, struct buffer_head *b | |
72 | goto out; | |
73 | ||
74 | reread: | |
75 | - read_lock(&pointers_lock); | |
76 | partial = get_branch(inode, depth, offsets, chain, &err); | |
77 | - read_unlock(&pointers_lock); | |
78 | ||
79 | /* Simplest case - block found, no allocation needed */ | |
80 | if (!partial) { | |
81 | @@ -291,9 +289,9 @@ static Indirect *find_shared(struct inode *inode, | |
82 | *top = 0; | |
83 | for (k = depth; k > 1 && !offsets[k-1]; k--) | |
84 | ; | |
85 | + partial = get_branch(inode, k, offsets, chain, &err); | |
86 | ||
87 | write_lock(&pointers_lock); | |
88 | - partial = get_branch(inode, k, offsets, chain, &err); | |
89 | if (!partial) | |
90 | partial = chain + k-1; | |
91 | /* | |
92 | -- | |
93 | 2.43.0 | |
94 |