1 From 419d6efc50e94bcf5d6b35cd8c71f79edadec564 Mon Sep 17 00:00:00 2001
2 From: Gao Xiang <gaoxiang25@huawei.com>
3 Date: Fri, 1 Feb 2019 20:16:31 +0800
4 Subject: staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
6 From: Gao Xiang <gaoxiang25@huawei.com>
8 commit 419d6efc50e94bcf5d6b35cd8c71f79edadec564 upstream.
11 ... and while we are at it, what happens to
12 unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
13 unsigned int matched = min(startprfx, endprfx);
15 struct qstr dname = QSTR_INIT(data + nameoff,
16 unlikely(mid >= ndirents - 1) ?
18 le16_to_cpu(de[mid + 1].nameoff) - nameoff);
20 /* string comparison without already matched prefix */
21 int ret = dirnamecmp(name, &dname, &matched);
22 if le16_to_cpu(de[...].nameoff) is not monotonically increasing? I.e.
23 what's to prevent e.g. (unsigned)-1 ending up in dname.len?
25 Corrupted fs image shouldn't oops the kernel.. "
27 Revisit the related lookup flow to address the issue.
29 Fixes: d72d1ce60174 ("staging: erofs: add namei functions")
30 Cc: <stable@vger.kernel.org> # 4.19+
31 Suggested-by: Al Viro <viro@ZenIV.linux.org.uk>
32 Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
33 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
37 drivers/staging/erofs/namei.c | 183 ++++++++++++++++++++++--------------------
38 1 file changed, 97 insertions(+), 86 deletions(-)
40 --- a/drivers/staging/erofs/namei.c
41 +++ b/drivers/staging/erofs/namei.c
44 #include <trace/events/erofs.h>
46 -/* based on the value of qn->len is accurate */
47 -static inline int dirnamecmp(struct qstr *qn,
48 - struct qstr *qd, unsigned int *matched)
50 + const unsigned char *name;
51 + const unsigned char *end;
54 +/* based on the end of qn is accurate and it must have the trailing '\0' */
55 +static inline int dirnamecmp(const struct erofs_qstr *qn,
56 + const struct erofs_qstr *qd,
57 + unsigned int *matched)
59 - unsigned int i = *matched, len = min(qn->len, qd->len);
61 - if (unlikely(i >= len)) {
63 - if (qn->len < qd->len) {
65 - * actually (qn->len == qd->len)
66 - * when qd->name[i] == '\0'
68 - return qd->name[i] == '\0' ? 0 : -1;
70 - return (qn->len > qd->len);
72 + unsigned int i = *matched;
74 - if (qn->name[i] != qd->name[i]) {
76 - return qn->name[i] > qd->name[i] ? 1 : -1;
78 + * on-disk error, let's only BUG_ON in the debugging mode.
79 + * otherwise, it will return 1 to just skip the invalid name
80 + * and go on (in consideration of the lookup performance).
82 + DBG_BUGON(qd->name > qd->end);
84 + /* qd could not have trailing '\0' */
85 + /* However it is absolutely safe if < qd->end */
86 + while (qd->name + i < qd->end && qd->name[i] != '\0') {
87 + if (qn->name[i] != qd->name[i]) {
89 + return qn->name[i] > qd->name[i] ? 1 : -1;
97 + /* See comments in __d_alloc on the terminating NUL character */
98 + return qn->name[i] == '\0' ? 0 : 1;
101 -static struct erofs_dirent *find_target_dirent(
103 - u8 *data, int maxsize)
104 +#define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1))
106 +static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
108 + unsigned int dirblksize,
109 + const int ndirents)
111 - unsigned int ndirents, head, back;
113 unsigned int startprfx, endprfx;
114 struct erofs_dirent *const de = (struct erofs_dirent *)data;
116 - /* make sure that maxsize is valid */
117 - BUG_ON(maxsize < sizeof(struct erofs_dirent));
119 - ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
121 - /* corrupted dir (may be unnecessary...) */
125 + /* since the 1st dirent has been evaluated previously */
128 startprfx = endprfx = 0;
130 while (head <= back) {
131 - unsigned int mid = head + (back - head) / 2;
132 - unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
133 + const int mid = head + (back - head) / 2;
134 + const int nameoff = nameoff_from_disk(de[mid].nameoff,
136 unsigned int matched = min(startprfx, endprfx);
138 - struct qstr dname = QSTR_INIT(data + nameoff,
139 - unlikely(mid >= ndirents - 1) ?
140 - maxsize - nameoff :
141 - le16_to_cpu(de[mid + 1].nameoff) - nameoff);
142 + struct erofs_qstr dname = {
143 + .name = data + nameoff,
144 + .end = unlikely(mid >= ndirents - 1) ?
145 + data + dirblksize :
146 + data + nameoff_from_disk(de[mid + 1].nameoff,
150 /* string comparison without already matched prefix */
151 int ret = dirnamecmp(name, &dname, &matched);
153 - if (unlikely(!ret))
154 + if (unlikely(!ret)) {
156 - else if (ret > 0) {
157 + } else if (ret > 0) {
160 - } else if (unlikely(mid < 1)) /* fix "mid" overflow */
167 @@ -91,12 +94,12 @@ static struct erofs_dirent *find_target_
168 return ERR_PTR(-ENOENT);
171 -static struct page *find_target_block_classic(
173 - struct qstr *name, int *_diff)
174 +static struct page *find_target_block_classic(struct inode *dir,
175 + struct erofs_qstr *name,
178 unsigned int startprfx, endprfx;
179 - unsigned int head, back;
181 struct address_space *const mapping = dir->i_mapping;
182 struct page *candidate = ERR_PTR(-ENOENT);
184 @@ -105,41 +108,43 @@ static struct page *find_target_block_cl
185 back = inode_datablocks(dir) - 1;
187 while (head <= back) {
188 - unsigned int mid = head + (back - head) / 2;
189 + const int mid = head + (back - head) / 2;
190 struct page *page = read_mapping_page(mapping, mid, NULL);
192 - if (IS_ERR(page)) {
194 - if (!IS_ERR(candidate)) /* valid candidate */
195 - put_page(candidate);
199 - unsigned int ndirents, matched;
201 + if (!IS_ERR(page)) {
202 struct erofs_dirent *de = kmap_atomic(page);
203 - unsigned int nameoff = le16_to_cpu(de->nameoff);
205 - ndirents = nameoff / sizeof(*de);
206 + const int nameoff = nameoff_from_disk(de->nameoff,
208 + const int ndirents = nameoff / sizeof(*de);
210 + unsigned int matched;
211 + struct erofs_qstr dname;
213 - /* corrupted dir (should have one entry at least) */
214 - BUG_ON(!ndirents || nameoff > PAGE_SIZE);
215 + if (unlikely(!ndirents)) {
219 + page = ERR_PTR(-EIO);
223 matched = min(startprfx, endprfx);
225 dname.name = (u8 *)de + nameoff;
226 - dname.len = ndirents == 1 ?
227 - /* since the rest of the last page is 0 */
228 - EROFS_BLKSIZ - nameoff
229 - : le16_to_cpu(de[1].nameoff) - nameoff;
231 + dname.end = (u8 *)de + EROFS_BLKSIZ;
233 + dname.end = (u8 *)de +
234 + nameoff_from_disk(de[1].nameoff,
237 /* string comparison without already matched prefix */
238 diff = dirnamecmp(name, &dname, &matched);
241 if (unlikely(!diff)) {
246 } else if (diff > 0) {
249 @@ -147,45 +152,51 @@ exact_out:
250 if (likely(!IS_ERR(candidate)))
253 + *_ndirents = ndirents;
257 - if (unlikely(mid < 1)) /* fix "mid" overflow */
265 +out: /* free if the candidate is valid */
266 + if (!IS_ERR(candidate))
267 + put_page(candidate);
274 int erofs_namei(struct inode *dir,
276 - erofs_nid_t *nid, unsigned int *d_type)
278 + erofs_nid_t *nid, unsigned int *d_type)
285 struct erofs_dirent *de;
286 + struct erofs_qstr qn;
288 if (unlikely(!dir->i_size))
292 - page = find_target_block_classic(dir, name, &diff);
293 + qn.name = name->name;
294 + qn.end = name->name + name->len;
297 + page = find_target_block_classic(dir, &qn, &ndirents);
299 if (unlikely(IS_ERR(page)))
300 return PTR_ERR(page);
302 data = kmap_atomic(page);
303 /* the target page has been mapped */
304 - de = likely(diff) ?
305 - /* since the rest of the last page is 0 */
306 - find_target_dirent(name, data, EROFS_BLKSIZ) :
307 - (struct erofs_dirent *)data;
309 + de = find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents);
311 + de = (struct erofs_dirent *)data;
313 if (likely(!IS_ERR(de))) {
314 *nid = le64_to_cpu(de->nid);