| Message ID | 1357371781-18194-1-git-send-email-yan@linux.vnet.ibm.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Sat, Jan 05, 2013 at 03:42:59PM +0800, Guo Chao wrote: > In ext4_dx_csum_verify(), if we detect corrupted data, > we do not compare checksum because checksum itself may > be wrong, but we should report error in this case. > > Cc: Darrick J. Wong <darrick.wong@oracle.com> > Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com> > --- > fs/ext4/namei.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index cac4482..843e29f 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -370,14 +370,14 @@ static int ext4_dx_csum_verify(struct inode *inode, > c = get_dx_countlimit(inode, dirent, &count_offset); > if (!c) { > EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D."); > - return 1; > + return 0; > } > limit = le16_to_cpu(c->limit); > count = le16_to_cpu(c->count); > if (count_offset + (limit * sizeof(struct dx_entry)) > > EXT4_BLOCK_SIZE(inode->i_sb) - sizeof(struct dx_tail)) { > warn_no_space_for_csum(inode); > - return 1; > + return 0; In both of these cases we cannot figure out where the dx block checksum lives, and therefore we have no stored checksum to compare against. This can result from enabling checksums on a existing filesystem and ignoring tune2fs' request to run fsck -D to rebuild dx blocks that are completely full. However, since we haven't a checksum that we could use to decide if there's real corruption, there's no cause to return -EIO to the user. Therefore, we print a warning and trust the sanity checks to catch totally bogus blocks, which is the best we can hope for. Sorry, but this doesn't seem necessary. --D > } > t = (struct dx_tail *)(((struct dx_entry *)c) + limit); > > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Darrick, On Sat, Jan 05, 2013 at 11:50:54AM -0800, Darrick J. Wong wrote: > On Sat, Jan 05, 2013 at 03:42:59PM +0800, Guo Chao wrote: > > In ext4_dx_csum_verify(), if we detect corrupted data, > > we do not compare checksum because checksum itself may > > be wrong, but we should report error in this case. > > > > Cc: Darrick J. Wong <darrick.wong@oracle.com> > > Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com> > > --- > > fs/ext4/namei.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > > index cac4482..843e29f 100644 > > --- a/fs/ext4/namei.c > > +++ b/fs/ext4/namei.c > > @@ -370,14 +370,14 @@ static int ext4_dx_csum_verify(struct inode *inode, > > c = get_dx_countlimit(inode, dirent, &count_offset); > > if (!c) { > > EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D."); > > - return 1; > > + return 0; > > } > > limit = le16_to_cpu(c->limit); > > count = le16_to_cpu(c->count); > > if (count_offset + (limit * sizeof(struct dx_entry)) > > > EXT4_BLOCK_SIZE(inode->i_sb) - sizeof(struct dx_tail)) { > > warn_no_space_for_csum(inode); > > - return 1; > > + return 0; > > In both of these cases we cannot figure out where the dx block checksum lives, > and therefore we have no stored checksum to compare against. This can result > from enabling checksums on a existing filesystem and ignoring tune2fs' request > to run fsck -D to rebuild dx blocks that are completely full. However, since > we haven't a checksum that we could use to decide if there's real corruption, > there's no cause to return -EIO to the user. Therefore, we print a warning and > trust the sanity checks to catch totally bogus blocks, which is the best we can > hope for. > > Sorry, but this doesn't seem necessary. Thanks for the explaination. I think ext4_dirent_csum_verify() can encounter similar problem but return error. But I'm not sure it's the same case. Thanks, Guo Chao > --D > > } > > t = (struct dx_tail *)(((struct dx_entry *)c) + limit); > > > > -- > > 1.7.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index cac4482..843e29f 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -370,14 +370,14 @@ static int ext4_dx_csum_verify(struct inode *inode, c = get_dx_countlimit(inode, dirent, &count_offset); if (!c) { EXT4_ERROR_INODE(inode, "dir seems corrupt? Run e2fsck -D."); - return 1; + return 0; } limit = le16_to_cpu(c->limit); count = le16_to_cpu(c->count); if (count_offset + (limit * sizeof(struct dx_entry)) > EXT4_BLOCK_SIZE(inode->i_sb) - sizeof(struct dx_tail)) { warn_no_space_for_csum(inode); - return 1; + return 0; } t = (struct dx_tail *)(((struct dx_entry *)c) + limit);
In ext4_dx_csum_verify(), if we detect corrupted data, we do not compare checksum because checksum itself may be wrong, but we should report error in this case. Cc: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com> --- fs/ext4/namei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)