| Message ID | 20190813023054.73126-3-gaoxiang25@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] staging: erofs: inline erofs_inode_is_data_compressed() | expand |
On 2019/8/13 10:30, Gao Xiang wrote: > Kill all the remaining BUG_ON in EROFS: > - one BUG_ON was used to detect xattr on-disk corruption, > proper error handling should be added for it instead; > - the other BUG_ONs are used to detect potential issues, > use DBG_BUGON only in (eng) debugging version. BTW, do we need add WARN_ON() into DBG_BUGON() to show some details function or call stack in where we encounter the issue? > > Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks,
Hi Chao, On Tue, Aug 13, 2019 at 11:20:22AM +0800, Chao Yu wrote: > On 2019/8/13 10:30, Gao Xiang wrote: > > Kill all the remaining BUG_ON in EROFS: > > - one BUG_ON was used to detect xattr on-disk corruption, > > proper error handling should be added for it instead; > > - the other BUG_ONs are used to detect potential issues, > > use DBG_BUGON only in (eng) debugging version. > > BTW, do we need add WARN_ON() into DBG_BUGON() to show some details function or > call stack in where we encounter the issue? Thanks for kindly review :) Agreed, it seems much better. If there are no other considerations here, I can submit another patch addressing it later or maybe we can change it in the next linux version since I'd like to focusing on moving out of staging for this round... Thanks, Gao Xiang > > > > > Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> > > Reviewed-by: Chao Yu <yuchao0@huawei.com> > > Thanks,
Hi Xiang, On 2019/8/13 11:57, Gao Xiang wrote: > Hi Chao, > > On Tue, Aug 13, 2019 at 11:20:22AM +0800, Chao Yu wrote: >> On 2019/8/13 10:30, Gao Xiang wrote: >>> Kill all the remaining BUG_ON in EROFS: >>> - one BUG_ON was used to detect xattr on-disk corruption, >>> proper error handling should be added for it instead; >>> - the other BUG_ONs are used to detect potential issues, >>> use DBG_BUGON only in (eng) debugging version. >> >> BTW, do we need add WARN_ON() into DBG_BUGON() to show some details function or >> call stack in where we encounter the issue? > > Thanks for kindly review :) > > Agreed, it seems much better. If there are no other considerations > here, I can submit another patch addressing it later or maybe we > can change it in the next linux version since I'd like to focusing > on moving out of staging for this round... No problem, we can change it in a proper time. Thanks, > > Thanks, > Gao Xiang > >> >>> >>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> >> >> Reviewed-by: Chao Yu <yuchao0@huawei.com> >> >> Thanks, > . >
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c index b29177a17347..289c7850ec96 100644 --- a/drivers/staging/erofs/xattr.c +++ b/drivers/staging/erofs/xattr.c @@ -115,7 +115,7 @@ static int init_inode_xattrs(struct inode *inode) for (i = 0; i < vi->xattr_shared_count; ++i) { if (unlikely(it.ofs >= EROFS_BLKSIZ)) { /* cannot be unaligned */ - BUG_ON(it.ofs != EROFS_BLKSIZ); + DBG_BUGON(it.ofs != EROFS_BLKSIZ); xattr_iter_end(&it, atomic_map); it.page = erofs_get_meta_page(sb, ++it.blkaddr, @@ -191,7 +191,7 @@ static int inline_xattr_iter_begin(struct xattr_iter *it, xattr_header_sz = inlinexattr_header_size(inode); if (unlikely(xattr_header_sz >= vi->xattr_isize)) { - BUG_ON(xattr_header_sz > vi->xattr_isize); + DBG_BUGON(xattr_header_sz > vi->xattr_isize); return -ENOATTR; } @@ -234,7 +234,11 @@ static int xattr_foreach(struct xattr_iter *it, if (tlimit) { unsigned int entry_sz = EROFS_XATTR_ENTRY_SIZE(&entry); - BUG_ON(*tlimit < entry_sz); + /* xattr on-disk corruption: xattr entry beyond xattr_isize */ + if (unlikely(*tlimit < entry_sz)) { + DBG_BUGON(1); + return -EIO; + } *tlimit -= entry_sz; } @@ -253,7 +257,7 @@ static int xattr_foreach(struct xattr_iter *it, while (processed < entry.e_name_len) { if (it->ofs >= EROFS_BLKSIZ) { - BUG_ON(it->ofs > EROFS_BLKSIZ); + DBG_BUGON(it->ofs > EROFS_BLKSIZ); err = xattr_iter_fixup(it); if (err) @@ -288,7 +292,7 @@ static int xattr_foreach(struct xattr_iter *it, while (processed < value_sz) { if (it->ofs >= EROFS_BLKSIZ) { - BUG_ON(it->ofs > EROFS_BLKSIZ); + DBG_BUGON(it->ofs > EROFS_BLKSIZ); err = xattr_iter_fixup(it); if (err)
Kill all the remaining BUG_ON in EROFS: - one BUG_ON was used to detect xattr on-disk corruption, proper error handling should be added for it instead; - the other BUG_ONs are used to detect potential issues, use DBG_BUGON only in (eng) debugging version. Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> --- drivers/staging/erofs/xattr.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)