Commit 419d6efc authored by Gao Xiang's avatar Gao Xiang Committed by Greg Kroah-Hartman

staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()

As Al pointed out, "
... and while we are at it, what happens to
	unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
	unsigned int matched = min(startprfx, endprfx);

	struct qstr dname = QSTR_INIT(data + nameoff,
		unlikely(mid >= ndirents - 1) ?
			maxsize - nameoff :
			le16_to_cpu(de[mid + 1].nameoff) - nameoff);

	/* string comparison without already matched prefix */
	int ret = dirnamecmp(name, &dname, &matched);
if le16_to_cpu(de[...].nameoff) is not monotonically increasing?  I.e.
what's to prevent e.g. (unsigned)-1 ending up in dname.len?

Corrupted fs image shouldn't oops the kernel.. "

Revisit the related lookup flow to address the issue.

Fixes: d72d1ce6 ("staging: erofs: add namei functions")
Cc: <stable@vger.kernel.org> # 4.19+
Suggested-by: default avatarAl Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: default avatarGao Xiang <gaoxiang25@huawei.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 18f2153d
...@@ -15,74 +15,77 @@ ...@@ -15,74 +15,77 @@
#include <trace/events/erofs.h> #include <trace/events/erofs.h>
/* based on the value of qn->len is accurate */ struct erofs_qstr {
static inline int dirnamecmp(struct qstr *qn, const unsigned char *name;
struct qstr *qd, unsigned int *matched) const unsigned char *end;
};
/* based on the end of qn is accurate and it must have the trailing '\0' */
static inline int dirnamecmp(const struct erofs_qstr *qn,
const struct erofs_qstr *qd,
unsigned int *matched)
{ {
unsigned int i = *matched, len = min(qn->len, qd->len); unsigned int i = *matched;
loop:
if (unlikely(i >= len)) {
*matched = i;
if (qn->len < qd->len) {
/* /*
* actually (qn->len == qd->len) * on-disk error, let's only BUG_ON in the debugging mode.
* when qd->name[i] == '\0' * otherwise, it will return 1 to just skip the invalid name
* and go on (in consideration of the lookup performance).
*/ */
return qd->name[i] == '\0' ? 0 : -1; DBG_BUGON(qd->name > qd->end);
}
return (qn->len > qd->len);
}
/* qd could not have trailing '\0' */
/* However it is absolutely safe if < qd->end */
while (qd->name + i < qd->end && qd->name[i] != '\0') {
if (qn->name[i] != qd->name[i]) { if (qn->name[i] != qd->name[i]) {
*matched = i; *matched = i;
return qn->name[i] > qd->name[i] ? 1 : -1; return qn->name[i] > qd->name[i] ? 1 : -1;
} }
++i; ++i;
goto loop; }
*matched = i;
/* See comments in __d_alloc on the terminating NUL character */
return qn->name[i] == '\0' ? 0 : 1;
} }
static struct erofs_dirent *find_target_dirent( #define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1))
struct qstr *name,
u8 *data, int maxsize) static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
u8 *data,
unsigned int dirblksize,
const int ndirents)
{ {
unsigned int ndirents, head, back; int head, back;
unsigned int startprfx, endprfx; unsigned int startprfx, endprfx;
struct erofs_dirent *const de = (struct erofs_dirent *)data; struct erofs_dirent *const de = (struct erofs_dirent *)data;
/* make sure that maxsize is valid */ /* since the 1st dirent has been evaluated previously */
BUG_ON(maxsize < sizeof(struct erofs_dirent)); head = 1;
ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
/* corrupted dir (may be unnecessary...) */
BUG_ON(!ndirents);
head = 0;
back = ndirents - 1; back = ndirents - 1;
startprfx = endprfx = 0; startprfx = endprfx = 0;
while (head <= back) { while (head <= back) {
unsigned int mid = head + (back - head) / 2; const int mid = head + (back - head) / 2;
unsigned int nameoff = le16_to_cpu(de[mid].nameoff); const int nameoff = nameoff_from_disk(de[mid].nameoff,
dirblksize);
unsigned int matched = min(startprfx, endprfx); unsigned int matched = min(startprfx, endprfx);
struct erofs_qstr dname = {
struct qstr dname = QSTR_INIT(data + nameoff, .name = data + nameoff,
unlikely(mid >= ndirents - 1) ? .end = unlikely(mid >= ndirents - 1) ?
maxsize - nameoff : data + dirblksize :
le16_to_cpu(de[mid + 1].nameoff) - nameoff); data + nameoff_from_disk(de[mid + 1].nameoff,
dirblksize)
};
/* string comparison without already matched prefix */ /* string comparison without already matched prefix */
int ret = dirnamecmp(name, &dname, &matched); int ret = dirnamecmp(name, &dname, &matched);
if (unlikely(!ret)) if (unlikely(!ret)) {
return de + mid; return de + mid;
else if (ret > 0) { } else if (ret > 0) {
head = mid + 1; head = mid + 1;
startprfx = matched; startprfx = matched;
} else if (unlikely(mid < 1)) /* fix "mid" overflow */ } else {
break;
else {
back = mid - 1; back = mid - 1;
endprfx = matched; endprfx = matched;
} }
...@@ -91,12 +94,12 @@ static struct erofs_dirent *find_target_dirent( ...@@ -91,12 +94,12 @@ static struct erofs_dirent *find_target_dirent(
return ERR_PTR(-ENOENT); return ERR_PTR(-ENOENT);
} }
static struct page *find_target_block_classic( static struct page *find_target_block_classic(struct inode *dir,
struct inode *dir, struct erofs_qstr *name,
struct qstr *name, int *_diff) int *_ndirents)
{ {
unsigned int startprfx, endprfx; unsigned int startprfx, endprfx;
unsigned int head, back; int head, back;
struct address_space *const mapping = dir->i_mapping; struct address_space *const mapping = dir->i_mapping;
struct page *candidate = ERR_PTR(-ENOENT); struct page *candidate = ERR_PTR(-ENOENT);
...@@ -105,41 +108,43 @@ static struct page *find_target_block_classic( ...@@ -105,41 +108,43 @@ static struct page *find_target_block_classic(
back = inode_datablocks(dir) - 1; back = inode_datablocks(dir) - 1;
while (head <= back) { while (head <= back) {
unsigned int mid = head + (back - head) / 2; const int mid = head + (back - head) / 2;
struct page *page = read_mapping_page(mapping, mid, NULL); struct page *page = read_mapping_page(mapping, mid, NULL);
if (IS_ERR(page)) { if (!IS_ERR(page)) {
exact_out:
if (!IS_ERR(candidate)) /* valid candidate */
put_page(candidate);
return page;
} else {
int diff;
unsigned int ndirents, matched;
struct qstr dname;
struct erofs_dirent *de = kmap_atomic(page); struct erofs_dirent *de = kmap_atomic(page);
unsigned int nameoff = le16_to_cpu(de->nameoff); const int nameoff = nameoff_from_disk(de->nameoff,
EROFS_BLKSIZ);
ndirents = nameoff / sizeof(*de); const int ndirents = nameoff / sizeof(*de);
int diff;
unsigned int matched;
struct erofs_qstr dname;
/* corrupted dir (should have one entry at least) */ if (unlikely(!ndirents)) {
BUG_ON(!ndirents || nameoff > PAGE_SIZE); DBG_BUGON(1);
kunmap_atomic(de);
put_page(page);
page = ERR_PTR(-EIO);
goto out;
}
matched = min(startprfx, endprfx); matched = min(startprfx, endprfx);
dname.name = (u8 *)de + nameoff; dname.name = (u8 *)de + nameoff;
dname.len = ndirents == 1 ? if (ndirents == 1)
/* since the rest of the last page is 0 */ dname.end = (u8 *)de + EROFS_BLKSIZ;
EROFS_BLKSIZ - nameoff else
: le16_to_cpu(de[1].nameoff) - nameoff; dname.end = (u8 *)de +
nameoff_from_disk(de[1].nameoff,
EROFS_BLKSIZ);
/* string comparison without already matched prefix */ /* string comparison without already matched prefix */
diff = dirnamecmp(name, &dname, &matched); diff = dirnamecmp(name, &dname, &matched);
kunmap_atomic(de); kunmap_atomic(de);
if (unlikely(!diff)) { if (unlikely(!diff)) {
*_diff = 0; *_ndirents = 0;
goto exact_out; goto out;
} else if (diff > 0) { } else if (diff > 0) {
head = mid + 1; head = mid + 1;
startprfx = matched; startprfx = matched;
...@@ -147,18 +152,20 @@ static struct page *find_target_block_classic( ...@@ -147,18 +152,20 @@ static struct page *find_target_block_classic(
if (!IS_ERR(candidate)) if (!IS_ERR(candidate))
put_page(candidate); put_page(candidate);
candidate = page; candidate = page;
*_ndirents = ndirents;
} else { } else {
put_page(page); put_page(page);
if (unlikely(mid < 1)) /* fix "mid" overflow */
break;
back = mid - 1; back = mid - 1;
endprfx = matched; endprfx = matched;
} }
continue;
} }
out: /* free if the candidate is valid */
if (!IS_ERR(candidate))
put_page(candidate);
return page;
} }
*_diff = 1;
return candidate; return candidate;
} }
...@@ -166,26 +173,30 @@ int erofs_namei(struct inode *dir, ...@@ -166,26 +173,30 @@ int erofs_namei(struct inode *dir,
struct qstr *name, struct qstr *name,
erofs_nid_t *nid, unsigned int *d_type) erofs_nid_t *nid, unsigned int *d_type)
{ {
int diff; int ndirents;
struct page *page; struct page *page;
u8 *data; void *data;
struct erofs_dirent *de; struct erofs_dirent *de;
struct erofs_qstr qn;
if (unlikely(!dir->i_size)) if (unlikely(!dir->i_size))
return -ENOENT; return -ENOENT;
diff = 1; qn.name = name->name;
page = find_target_block_classic(dir, name, &diff); qn.end = name->name + name->len;
ndirents = 0;
page = find_target_block_classic(dir, &qn, &ndirents);
if (IS_ERR(page)) if (IS_ERR(page))
return PTR_ERR(page); return PTR_ERR(page);
data = kmap_atomic(page); data = kmap_atomic(page);
/* the target page has been mapped */ /* the target page has been mapped */
de = likely(diff) ? if (ndirents)
/* since the rest of the last page is 0 */ de = find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents);
find_target_dirent(name, data, EROFS_BLKSIZ) : else
(struct erofs_dirent *)data; de = (struct erofs_dirent *)data;
if (!IS_ERR(de)) { if (!IS_ERR(de)) {
*nid = le64_to_cpu(de->nid); *nid = le64_to_cpu(de->nid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment