]>
Commit | Line | Data |
---|---|---|
6ebfa1bb GKH |
1 | From foo@baz Tue Mar 12 05:46:41 PDT 2019 |
2 | From: Gao Xiang <gaoxiang25@huawei.com> | |
3 | Date: Mon, 11 Mar 2019 14:08:58 +0800 | |
4 | Subject: staging: erofs: keep corrupted fs from crashing kernel in erofs_namei() | |
5 | To: <stable@vger.kernel.org> | |
6 | Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, LKML <linux-kernel@vger.kernel.org>, <linux-erofs@lists.ozlabs.org>, Chao Yu <yuchao0@huawei.com>, Chao Yu <chao@kernel.org>, Miao Xie <miaoxie@huawei.com>, Fang Wei <fangwei1@huawei.com>, Gao Xiang <gaoxiang25@huawei.com> | |
7 | Message-ID: <20190311060858.28654-5-gaoxiang25@huawei.com> | |
8 | ||
9 | From: Gao Xiang <gaoxiang25@huawei.com> | |
10 | ||
11 | commit 419d6efc50e94bcf5d6b35cd8c71f79edadec564 upstream. | |
12 | ||
13 | As Al pointed out, " | |
14 | ... and while we are at it, what happens to | |
15 | unsigned int nameoff = le16_to_cpu(de[mid].nameoff); | |
16 | unsigned int matched = min(startprfx, endprfx); | |
17 | ||
18 | struct qstr dname = QSTR_INIT(data + nameoff, | |
19 | unlikely(mid >= ndirents - 1) ? | |
20 | maxsize - nameoff : | |
21 | le16_to_cpu(de[mid + 1].nameoff) - nameoff); | |
22 | ||
23 | /* string comparison without already matched prefix */ | |
24 | int ret = dirnamecmp(name, &dname, &matched); | |
25 | if le16_to_cpu(de[...].nameoff) is not monotonically increasing? I.e. | |
26 | what's to prevent e.g. (unsigned)-1 ending up in dname.len? | |
27 | ||
28 | Corrupted fs image shouldn't oops the kernel.. " | |
29 | ||
30 | Revisit the related lookup flow to address the issue. | |
31 | ||
32 | Fixes: d72d1ce60174 ("staging: erofs: add namei functions") | |
33 | Cc: <stable@vger.kernel.org> # 4.19+ | |
34 | Suggested-by: Al Viro <viro@ZenIV.linux.org.uk> | |
35 | Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> | |
36 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
37 | --- | |
38 | drivers/staging/erofs/namei.c | 189 ++++++++++++++++++++++-------------------- | |
39 | 1 file changed, 100 insertions(+), 89 deletions(-) | |
40 | ||
41 | --- a/drivers/staging/erofs/namei.c | |
42 | +++ b/drivers/staging/erofs/namei.c | |
43 | @@ -15,74 +15,77 @@ | |
44 | ||
45 | #include <trace/events/erofs.h> | |
46 | ||
47 | -/* based on the value of qn->len is accurate */ | |
48 | -static inline int dirnamecmp(struct qstr *qn, | |
49 | - struct qstr *qd, unsigned *matched) | |
50 | +struct erofs_qstr { | |
51 | + const unsigned char *name; | |
52 | + const unsigned char *end; | |
53 | +}; | |
54 | + | |
55 | +/* based on the end of qn is accurate and it must have the trailing '\0' */ | |
56 | +static inline int dirnamecmp(const struct erofs_qstr *qn, | |
57 | + const struct erofs_qstr *qd, | |
58 | + unsigned int *matched) | |
59 | { | |
60 | - unsigned i = *matched, len = min(qn->len, qd->len); | |
61 | -loop: | |
62 | - if (unlikely(i >= len)) { | |
63 | - *matched = i; | |
64 | - if (qn->len < qd->len) { | |
65 | - /* | |
66 | - * actually (qn->len == qd->len) | |
67 | - * when qd->name[i] == '\0' | |
68 | - */ | |
69 | - return qd->name[i] == '\0' ? 0 : -1; | |
70 | - } | |
71 | - return (qn->len > qd->len); | |
72 | - } | |
73 | + unsigned int i = *matched; | |
74 | ||
75 | - if (qn->name[i] != qd->name[i]) { | |
76 | - *matched = i; | |
77 | - return qn->name[i] > qd->name[i] ? 1 : -1; | |
78 | + /* | |
79 | + * on-disk error, let's only BUG_ON in the debugging mode. | |
80 | + * otherwise, it will return 1 to just skip the invalid name | |
81 | + * and go on (in consideration of the lookup performance). | |
82 | + */ | |
83 | + DBG_BUGON(qd->name > qd->end); | |
84 | + | |
85 | + /* qd could not have trailing '\0' */ | |
86 | + /* However it is absolutely safe if < qd->end */ | |
87 | + while (qd->name + i < qd->end && qd->name[i] != '\0') { | |
88 | + if (qn->name[i] != qd->name[i]) { | |
89 | + *matched = i; | |
90 | + return qn->name[i] > qd->name[i] ? 1 : -1; | |
91 | + } | |
92 | + ++i; | |
93 | } | |
94 | - | |
95 | - ++i; | |
96 | - goto loop; | |
97 | + *matched = i; | |
98 | + /* See comments in __d_alloc on the terminating NUL character */ | |
99 | + return qn->name[i] == '\0' ? 0 : 1; | |
100 | } | |
101 | ||
102 | -static struct erofs_dirent *find_target_dirent( | |
103 | - struct qstr *name, | |
104 | - u8 *data, int maxsize) | |
105 | +#define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1)) | |
106 | + | |
107 | +static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name, | |
108 | + u8 *data, | |
109 | + unsigned int dirblksize, | |
110 | + const int ndirents) | |
111 | { | |
112 | - unsigned ndirents, head, back; | |
113 | - unsigned startprfx, endprfx; | |
114 | + int head, back; | |
115 | + unsigned int startprfx, endprfx; | |
116 | struct erofs_dirent *const de = (struct erofs_dirent *)data; | |
117 | ||
118 | - /* make sure that maxsize is valid */ | |
119 | - BUG_ON(maxsize < sizeof(struct erofs_dirent)); | |
120 | - | |
121 | - ndirents = le16_to_cpu(de->nameoff) / sizeof(*de); | |
122 | - | |
123 | - /* corrupted dir (may be unnecessary...) */ | |
124 | - BUG_ON(!ndirents); | |
125 | - | |
126 | - head = 0; | |
127 | + /* since the 1st dirent has been evaluated previously */ | |
128 | + head = 1; | |
129 | back = ndirents - 1; | |
130 | startprfx = endprfx = 0; | |
131 | ||
132 | while (head <= back) { | |
133 | - unsigned mid = head + (back - head) / 2; | |
134 | - unsigned nameoff = le16_to_cpu(de[mid].nameoff); | |
135 | - unsigned matched = min(startprfx, endprfx); | |
136 | - | |
137 | - struct qstr dname = QSTR_INIT(data + nameoff, | |
138 | - unlikely(mid >= ndirents - 1) ? | |
139 | - maxsize - nameoff : | |
140 | - le16_to_cpu(de[mid + 1].nameoff) - nameoff); | |
141 | + const int mid = head + (back - head) / 2; | |
142 | + const int nameoff = nameoff_from_disk(de[mid].nameoff, | |
143 | + dirblksize); | |
144 | + unsigned int matched = min(startprfx, endprfx); | |
145 | + struct erofs_qstr dname = { | |
146 | + .name = data + nameoff, | |
147 | + .end = unlikely(mid >= ndirents - 1) ? | |
148 | + data + dirblksize : | |
149 | + data + nameoff_from_disk(de[mid + 1].nameoff, | |
150 | + dirblksize) | |
151 | + }; | |
152 | ||
153 | /* string comparison without already matched prefix */ | |
154 | int ret = dirnamecmp(name, &dname, &matched); | |
155 | ||
156 | - if (unlikely(!ret)) | |
157 | + if (unlikely(!ret)) { | |
158 | return de + mid; | |
159 | - else if (ret > 0) { | |
160 | + } else if (ret > 0) { | |
161 | head = mid + 1; | |
162 | startprfx = matched; | |
163 | - } else if (unlikely(mid < 1)) /* fix "mid" overflow */ | |
164 | - break; | |
165 | - else { | |
166 | + } else { | |
167 | back = mid - 1; | |
168 | endprfx = matched; | |
169 | } | |
170 | @@ -91,12 +94,12 @@ static struct erofs_dirent *find_target_ | |
171 | return ERR_PTR(-ENOENT); | |
172 | } | |
173 | ||
174 | -static struct page *find_target_block_classic( | |
175 | - struct inode *dir, | |
176 | - struct qstr *name, int *_diff) | |
177 | +static struct page *find_target_block_classic(struct inode *dir, | |
178 | + struct erofs_qstr *name, | |
179 | + int *_ndirents) | |
180 | { | |
181 | - unsigned startprfx, endprfx; | |
182 | - unsigned head, back; | |
183 | + unsigned int startprfx, endprfx; | |
184 | + int head, back; | |
185 | struct address_space *const mapping = dir->i_mapping; | |
186 | struct page *candidate = ERR_PTR(-ENOENT); | |
187 | ||
188 | @@ -105,41 +108,43 @@ static struct page *find_target_block_cl | |
189 | back = inode_datablocks(dir) - 1; | |
190 | ||
191 | while (head <= back) { | |
192 | - unsigned mid = head + (back - head) / 2; | |
193 | + const int mid = head + (back - head) / 2; | |
194 | struct page *page = read_mapping_page(mapping, mid, NULL); | |
195 | ||
196 | - if (IS_ERR(page)) { | |
197 | -exact_out: | |
198 | - if (!IS_ERR(candidate)) /* valid candidate */ | |
199 | - put_page(candidate); | |
200 | - return page; | |
201 | - } else { | |
202 | - int diff; | |
203 | - unsigned ndirents, matched; | |
204 | - struct qstr dname; | |
205 | + if (!IS_ERR(page)) { | |
206 | struct erofs_dirent *de = kmap_atomic(page); | |
207 | - unsigned nameoff = le16_to_cpu(de->nameoff); | |
208 | - | |
209 | - ndirents = nameoff / sizeof(*de); | |
210 | + const int nameoff = nameoff_from_disk(de->nameoff, | |
211 | + EROFS_BLKSIZ); | |
212 | + const int ndirents = nameoff / sizeof(*de); | |
213 | + int diff; | |
214 | + unsigned int matched; | |
215 | + struct erofs_qstr dname; | |
216 | ||
217 | - /* corrupted dir (should have one entry at least) */ | |
218 | - BUG_ON(!ndirents || nameoff > PAGE_SIZE); | |
219 | + if (unlikely(!ndirents)) { | |
220 | + DBG_BUGON(1); | |
221 | + kunmap_atomic(de); | |
222 | + put_page(page); | |
223 | + page = ERR_PTR(-EIO); | |
224 | + goto out; | |
225 | + } | |
226 | ||
227 | matched = min(startprfx, endprfx); | |
228 | ||
229 | dname.name = (u8 *)de + nameoff; | |
230 | - dname.len = ndirents == 1 ? | |
231 | - /* since the rest of the last page is 0 */ | |
232 | - EROFS_BLKSIZ - nameoff | |
233 | - : le16_to_cpu(de[1].nameoff) - nameoff; | |
234 | + if (ndirents == 1) | |
235 | + dname.end = (u8 *)de + EROFS_BLKSIZ; | |
236 | + else | |
237 | + dname.end = (u8 *)de + | |
238 | + nameoff_from_disk(de[1].nameoff, | |
239 | + EROFS_BLKSIZ); | |
240 | ||
241 | /* string comparison without already matched prefix */ | |
242 | diff = dirnamecmp(name, &dname, &matched); | |
243 | kunmap_atomic(de); | |
244 | ||
245 | if (unlikely(!diff)) { | |
246 | - *_diff = 0; | |
247 | - goto exact_out; | |
248 | + *_ndirents = 0; | |
249 | + goto out; | |
250 | } else if (diff > 0) { | |
251 | head = mid + 1; | |
252 | startprfx = matched; | |
253 | @@ -147,45 +152,51 @@ exact_out: | |
254 | if (likely(!IS_ERR(candidate))) | |
255 | put_page(candidate); | |
256 | candidate = page; | |
257 | + *_ndirents = ndirents; | |
258 | } else { | |
259 | put_page(page); | |
260 | ||
261 | - if (unlikely(mid < 1)) /* fix "mid" overflow */ | |
262 | - break; | |
263 | - | |
264 | back = mid - 1; | |
265 | endprfx = matched; | |
266 | } | |
267 | + continue; | |
268 | } | |
269 | +out: /* free if the candidate is valid */ | |
270 | + if (!IS_ERR(candidate)) | |
271 | + put_page(candidate); | |
272 | + return page; | |
273 | } | |
274 | - *_diff = 1; | |
275 | return candidate; | |
276 | } | |
277 | ||
278 | int erofs_namei(struct inode *dir, | |
279 | - struct qstr *name, | |
280 | - erofs_nid_t *nid, unsigned *d_type) | |
281 | + struct qstr *name, | |
282 | + erofs_nid_t *nid, unsigned int *d_type) | |
283 | { | |
284 | - int diff; | |
285 | + int ndirents; | |
286 | struct page *page; | |
287 | - u8 *data; | |
288 | + void *data; | |
289 | struct erofs_dirent *de; | |
290 | + struct erofs_qstr qn; | |
291 | ||
292 | if (unlikely(!dir->i_size)) | |
293 | return -ENOENT; | |
294 | ||
295 | - diff = 1; | |
296 | - page = find_target_block_classic(dir, name, &diff); | |
297 | + qn.name = name->name; | |
298 | + qn.end = name->name + name->len; | |
299 | + | |
300 | + ndirents = 0; | |
301 | + page = find_target_block_classic(dir, &qn, &ndirents); | |
302 | ||
303 | if (unlikely(IS_ERR(page))) | |
304 | return PTR_ERR(page); | |
305 | ||
306 | data = kmap_atomic(page); | |
307 | /* the target page has been mapped */ | |
308 | - de = likely(diff) ? | |
309 | - /* since the rest of the last page is 0 */ | |
310 | - find_target_dirent(name, data, EROFS_BLKSIZ) : | |
311 | - (struct erofs_dirent *)data; | |
312 | + if (ndirents) | |
313 | + de = find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents); | |
314 | + else | |
315 | + de = (struct erofs_dirent *)data; | |
316 | ||
317 | if (likely(!IS_ERR(de))) { | |
318 | *nid = le64_to_cpu(de->nid); |