Skip to content

Commit 798badf

Browse files
gregkhgregkh
committed
Revert "staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()"
This reverts commit d4104c5. Turns out it still needs some more work, I merged it to soon :( Reported-by: Gao Xiang <gaoxiang25@huawei.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Cc: Al Viro <viro@ZenIV.linux.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d4104c5 commit 798badf

File tree

1 file changed

+78
-89
lines changed

1 file changed

+78
-89
lines changed

‎drivers/staging/erofs/namei.c

Lines changed: 78 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -15,76 +15,74 @@
1515

1616
#include <trace/events/erofs.h>
1717

18-
struct erofs_qstr {
19-
const unsigned char *name;
20-
const unsigned char *end;
21-
};
22-
23-
/* based on the end of qn is accurate and it must have the trailing '\0' */
24-
static inline int dirnamecmp(const struct erofs_qstr *qn,
25-
const struct erofs_qstr *qd,
26-
unsigned int *matched)
18+
/* based on the value of qn->len is accurate */
19+
static inline int dirnamecmp(struct qstr *qn,
20+
struct qstr *qd, unsigned int *matched)
2721
{
28-
unsigned int i = *matched;
29-
30-
/*
31-
* on-disk error, let's only BUG_ON in the debugging mode.
32-
* otherwise, it will return 1 to just skip the invalid name
33-
* and go on (in consideration of the lookup performance).
34-
*/
35-
DBG_BUGON(qd->name > qd->end);
36-
37-
/* qd could not have trailing '\0' */
38-
/* However it is absolutely safe if < qd->end */
39-
while (qd->name + i < qd->end && qd->name[i] != '\0') {
40-
if (qn->name[i] != qd->name[i]) {
41-
*matched = i;
42-
return qn->name[i] > qd->name[i] ? 1 : -1;
22+
unsigned int i = *matched, len = min(qn->len, qd->len);
23+
loop:
24+
if (unlikely(i >= len)) {
25+
*matched = i;
26+
if (qn->len < qd->len) {
27+
/*
28+
* actually (qn->len == qd->len)
29+
* when qd->name[i] == '\0'
30+
*/
31+
return qd->name[i] == '\0' ? 0 : -1;
4332
}
44-
++i;
33+
return (qn->len > qd->len);
34+
}
35+
36+
if (qn->name[i] != qd->name[i]) {
37+
*matched = i;
38+
return qn->name[i] > qd->name[i] ? 1 : -1;
4539
}
46-
*matched = i;
47-
/* See comments in __d_alloc on the terminating NUL character */
48-
return qn->name[i] == '\0' ? 0 : 1;
49-
}
5040

51-
#define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1))
41+
++i;
42+
goto loop;
43+
}
5244

53-
static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
54-
u8 *data,
55-
unsigned int dirblksize,
56-
const int ndirents)
45+
static struct erofs_dirent *find_target_dirent(
46+
struct qstr *name,
47+
u8 *data, int maxsize)
5748
{
58-
int head, back;
49+
unsigned int ndirents, head, back;
5950
unsigned int startprfx, endprfx;
6051
struct erofs_dirent *const de = (struct erofs_dirent *)data;
6152

53+
/* make sure that maxsize is valid */
54+
BUG_ON(maxsize < sizeof(struct erofs_dirent));
55+
56+
ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
57+
58+
/* corrupted dir (may be unnecessary...) */
59+
BUG_ON(!ndirents);
60+
6261
head = 0;
6362
back = ndirents - 1;
6463
startprfx = endprfx = 0;
6564

6665
while (head <= back) {
67-
const int mid = head + (back - head) / 2;
68-
const int nameoff = nameoff_from_disk(de[mid].nameoff,
69-
dirblksize);
66+
unsigned int mid = head + (back - head) / 2;
67+
unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
7068
unsigned int matched = min(startprfx, endprfx);
71-
struct erofs_qstr dname = {
72-
.name = data + nameoff,
73-
.end = unlikely(mid >= ndirents - 1) ?
74-
data + dirblksize :
75-
data + nameoff_from_disk(de[mid + 1].nameoff,
76-
dirblksize)
77-
};
69+
70+
struct qstr dname = QSTR_INIT(data + nameoff,
71+
unlikely(mid >= ndirents - 1) ?
72+
maxsize - nameoff :
73+
le16_to_cpu(de[mid + 1].nameoff) - nameoff);
7874

7975
/* string comparison without already matched prefix */
8076
int ret = dirnamecmp(name, &dname, &matched);
8177

82-
if (unlikely(!ret)) {
78+
if (unlikely(!ret))
8379
return de + mid;
84-
} else if (ret > 0) {
80+
else if (ret > 0) {
8581
head = mid + 1;
8682
startprfx = matched;
87-
} else {
83+
} else if (unlikely(mid < 1)) /* fix "mid" overflow */
84+
break;
85+
else {
8886
back = mid - 1;
8987
endprfx = matched;
9088
}
@@ -93,13 +91,12 @@ static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
9391
return ERR_PTR(-ENOENT);
9492
}
9593

96-
static struct page *find_target_block_classic(struct inode *dir,
97-
struct erofs_qstr *name,
98-
int *_diff,
99-
int *_ndirents)
94+
static struct page *find_target_block_classic(
95+
struct inode *dir,
96+
struct qstr *name, int *_diff)
10097
{
10198
unsigned int startprfx, endprfx;
102-
int head, back;
99+
unsigned int head, back;
103100
struct address_space *const mapping = dir->i_mapping;
104101
struct page *candidate = ERR_PTR(-ENOENT);
105102

@@ -108,85 +105,77 @@ static struct page *find_target_block_classic(struct inode *dir,
108105
back = inode_datablocks(dir) - 1;
109106

110107
while (head <= back) {
111-
const int mid = head + (back - head) / 2;
108+
unsigned int mid = head + (back - head) / 2;
112109
struct page *page = read_mapping_page(mapping, mid, NULL);
113110

114-
if (!IS_ERR(page)) {
115-
struct erofs_dirent *de = kmap_atomic(page);
116-
const int nameoff = nameoff_from_disk(de->nameoff,
117-
EROFS_BLKSIZ);
118-
const int ndirents = nameoff / sizeof(*de);
111+
if (IS_ERR(page)) {
112+
exact_out:
113+
if (!IS_ERR(candidate)) /* valid candidate */
114+
put_page(candidate);
115+
return page;
116+
} else {
119117
int diff;
120-
unsigned int matched;
121-
struct erofs_qstr dname;
118+
unsigned int ndirents, matched;
119+
struct qstr dname;
120+
struct erofs_dirent *de = kmap_atomic(page);
121+
unsigned int nameoff = le16_to_cpu(de->nameoff);
122122

123-
if (unlikely(!ndirents)) {
124-
DBG_BUGON(1);
125-
put_page(page);
126-
page = ERR_PTR(-EIO);
127-
goto out;
128-
}
123+
ndirents = nameoff / sizeof(*de);
124+
125+
/* corrupted dir (should have one entry at least) */
126+
BUG_ON(!ndirents || nameoff > PAGE_SIZE);
129127

130128
matched = min(startprfx, endprfx);
131129

132130
dname.name = (u8 *)de + nameoff;
133-
if (ndirents == 1)
134-
dname.end = (u8 *)de + EROFS_BLKSIZ;
135-
else
136-
dname.end = (u8 *)de +
137-
nameoff_from_disk(de[1].nameoff,
138-
EROFS_BLKSIZ);
131+
dname.len = ndirents == 1 ?
132+
/* since the rest of the last page is 0 */
133+
EROFS_BLKSIZ - nameoff
134+
: le16_to_cpu(de[1].nameoff) - nameoff;
139135

140136
/* string comparison without already matched prefix */
141137
diff = dirnamecmp(name, &dname, &matched);
142138
kunmap_atomic(de);
143139

144140
if (unlikely(!diff)) {
145141
*_diff = 0;
146-
goto out;
142+
goto exact_out;
147143
} else if (diff > 0) {
148144
head = mid + 1;
149145
startprfx = matched;
150146

151147
if (likely(!IS_ERR(candidate)))
152148
put_page(candidate);
153149
candidate = page;
154-
*_ndirents = ndirents;
155150
} else {
156151
put_page(page);
157152

153+
if (unlikely(mid < 1)) /* fix "mid" overflow */
154+
break;
155+
158156
back = mid - 1;
159157
endprfx = matched;
160158
}
161-
continue;
162159
}
163-
out: /* free if the candidate is valid */
164-
if (!IS_ERR(candidate))
165-
put_page(candidate);
166-
return page;
167160
}
168161
*_diff = 1;
169162
return candidate;
170163
}
171164

172165
int erofs_namei(struct inode *dir,
173-
struct qstr *name,
174-
erofs_nid_t *nid, unsigned int *d_type)
166+
struct qstr *name,
167+
erofs_nid_t *nid, unsigned int *d_type)
175168
{
176-
int diff, ndirents;
169+
int diff;
177170
struct page *page;
178171
u8 *data;
179172
struct erofs_dirent *de;
180-
struct erofs_qstr qn;
181173

182174
if (unlikely(!dir->i_size))
183175
return -ENOENT;
184176

185-
qn.name = name->name;
186-
qn.end = name->name + name->len;
187-
188177
diff = 1;
189-
page = find_target_block_classic(dir, &qn, &diff, &ndirents);
178+
page = find_target_block_classic(dir, name, &diff);
190179

191180
if (unlikely(IS_ERR(page)))
192181
return PTR_ERR(page);
@@ -195,7 +184,7 @@ int erofs_namei(struct inode *dir,
195184
/* the target page has been mapped */
196185
de = likely(diff) ?
197186
/* since the rest of the last page is 0 */
198-
find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents) :
187+
find_target_dirent(name, data, EROFS_BLKSIZ) :
199188
(struct erofs_dirent *)data;
200189

201190
if (likely(!IS_ERR(de))) {

0 commit comments

Comments
 (0)