* [PATCH 0/2] ext4/jbd2: misc 3.17 bugfixes, part 2 @ 2014-09-14 17:32 Darrick J. Wong 2014-09-14 17:32 ` [PATCH 1/2] ext4: check EA value offset when loading Darrick J. Wong 2014-09-14 17:33 ` [PATCH 2/2] jbd2: free bh when descriptor block checksum fails Darrick J. Wong 0 siblings, 2 replies; 6+ messages in thread From: Darrick J. Wong @ 2014-09-14 17:32 UTC (permalink / raw) To: tytso, darrick.wong; +Cc: linux-ext4 Hi all, Here are two more patches against 3.17-rc4 to fix some minor problems in jbd2 and ext4. Neither of these two depend on each other; they fix separate small bugs. The first patch teaches ext4 to check the e_value_offs value in the extended attribute data structure to avoid data corruption and kernel crashes resulting from the corruption. This fixes the crash I alluded to in the previous 3.17 bugfixes email pile. The second patch fixes a bh leak in the journal replay code when a descriptor block fails checksum verification. Patches are against 3.17-rc4, and have been xfstest'd. Comments and questions are, as always, welcome. --D ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] ext4: check EA value offset when loading 2014-09-14 17:32 [PATCH 0/2] ext4/jbd2: misc 3.17 bugfixes, part 2 Darrick J. Wong @ 2014-09-14 17:32 ` Darrick J. Wong 2014-09-16 18:41 ` Theodore Ts'o 2014-09-14 17:33 ` [PATCH 2/2] jbd2: free bh when descriptor block checksum fails Darrick J. Wong 1 sibling, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2014-09-14 17:32 UTC (permalink / raw) To: tytso, darrick.wong; +Cc: linux-ext4 When loading extended attributes, check each entry's value offset to make sure it doesn't collide with the entries. Without this check it is easy to crash the kernel by mounting a malicious FS containing a file with an EA wherein e_value_offs = 0 and e_value_size > 0 and then deleting the EA, which corrupts the name list. (See the f_ea_value_crash test's FS image in e2fsprogs for an example.) Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/ext4/xattr.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index e738733..c11738e 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -189,15 +189,29 @@ ext4_listxattr(struct dentry *dentry, char *buffer, size_t size) return ext4_xattr_list(dentry, buffer, size); } -static int -ext4_xattr_check_names(struct ext4_xattr_entry *entry, void *end) +int +ext4_xattr_check_names(struct ext4_xattr_entry *entry, void *end, + void *value_start) { - while (!IS_LAST_ENTRY(entry)) { - struct ext4_xattr_entry *next = EXT4_XATTR_NEXT(entry); + struct ext4_xattr_entry *e = entry; + + while (!IS_LAST_ENTRY(e)) { + struct ext4_xattr_entry *next = EXT4_XATTR_NEXT(e); if ((void *)next >= end) return -EIO; - entry = next; + e = next; } + + while (!IS_LAST_ENTRY(entry)) { + if (entry->e_value_size != 0 && + (value_start + le16_to_cpu(entry->e_value_offs) < + (void *)e + sizeof(__u32) || + value_start + le16_to_cpu(entry->e_value_offs) + + le32_to_cpu(entry->e_value_size) > end)) + return -EIO; + entry = EXT4_XATTR_NEXT(entry); + } + return 0; } @@ -214,7 +228,8 @@ ext4_xattr_check_block(struct inode *inode, struct buffer_head *bh) return -EIO; if (!ext4_xattr_block_csum_verify(inode, bh->b_blocknr, BHDR(bh))) return -EIO; - error = ext4_xattr_check_names(BFIRST(bh), bh->b_data + bh->b_size); + error = ext4_xattr_check_names(BFIRST(bh), bh->b_data + bh->b_size, + bh->b_data); if (!error) set_buffer_verified(bh); return error; @@ -331,7 +346,7 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name, header = IHDR(inode, raw_inode); entry = IFIRST(header); end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size; - error = ext4_xattr_check_names(entry, end); + error = ext4_xattr_check_names(entry, end, entry); if (error) goto cleanup; error = ext4_xattr_find_entry(&entry, name_index, name, @@ -463,7 +478,7 @@ ext4_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size) raw_inode = ext4_raw_inode(&iloc); header = IHDR(inode, raw_inode); end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size; - error = ext4_xattr_check_names(IFIRST(header), end); + error = ext4_xattr_check_names(IFIRST(header), end, IFIRST(header)); if (error) goto cleanup; error = ext4_xattr_list_entries(dentry, IFIRST(header), @@ -986,7 +1001,8 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i, is->s.here = is->s.first; is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size; if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) { - error = ext4_xattr_check_names(IFIRST(header), is->s.end); + error = ext4_xattr_check_names(IFIRST(header), is->s.end, + IFIRST(header)); if (error) return error; /* Find the named attribute. */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ext4: check EA value offset when loading 2014-09-14 17:32 ` [PATCH 1/2] ext4: check EA value offset when loading Darrick J. Wong @ 2014-09-16 18:41 ` Theodore Ts'o 0 siblings, 0 replies; 6+ messages in thread From: Theodore Ts'o @ 2014-09-16 18:41 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-ext4 On Sun, Sep 14, 2014 at 10:32:59AM -0700, Darrick J. Wong wrote: > When loading extended attributes, check each entry's value offset to > make sure it doesn't collide with the entries. > > Without this check it is easy to crash the kernel by mounting a > malicious FS containing a file with an EA wherein e_value_offs = 0 and > e_value_size > 0 and then deleting the EA, which corrupts the name > list. > > (See the f_ea_value_crash test's FS image in e2fsprogs for an example.) > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] jbd2: free bh when descriptor block checksum fails 2014-09-14 17:32 [PATCH 0/2] ext4/jbd2: misc 3.17 bugfixes, part 2 Darrick J. Wong 2014-09-14 17:32 ` [PATCH 1/2] ext4: check EA value offset when loading Darrick J. Wong @ 2014-09-14 17:33 ` Darrick J. Wong 2014-09-14 19:13 ` Eric Sandeen 1 sibling, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2014-09-14 17:33 UTC (permalink / raw) To: tytso, darrick.wong; +Cc: linux-ext4 Free the buffer head if the journal descriptor block fails checksum verification. This is the jbd2 port of the e2fsprogs patch "e2fsck: free bh on csum verify error in do_one_pass". Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/jbd2/recovery.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index 9b329b5..bcbef08 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -525,6 +525,7 @@ static int do_one_pass(journal_t *journal, !jbd2_descr_block_csum_verify(journal, bh->b_data)) { err = -EIO; + brelse(bh); goto failed; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] jbd2: free bh when descriptor block checksum fails 2014-09-14 17:33 ` [PATCH 2/2] jbd2: free bh when descriptor block checksum fails Darrick J. Wong @ 2014-09-14 19:13 ` Eric Sandeen 2014-09-16 18:43 ` Theodore Ts'o 0 siblings, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2014-09-14 19:13 UTC (permalink / raw) To: Darrick J. Wong, tytso; +Cc: linux-ext4 On 9/14/14 12:33 PM, Darrick J. Wong wrote: > Free the buffer head if the journal descriptor block fails checksum > verification. > > This is the jbd2 port of the e2fsprogs patch "e2fsck: free bh on csum > verify error in do_one_pass". > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > fs/jbd2/recovery.c | 1 + > 1 file changed, 1 insertion(+) > > > diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c > index 9b329b5..bcbef08 100644 > --- a/fs/jbd2/recovery.c > +++ b/fs/jbd2/recovery.c > @@ -525,6 +525,7 @@ static int do_one_pass(journal_t *journal, > !jbd2_descr_block_csum_verify(journal, > bh->b_data)) { > err = -EIO; > + brelse(bh); > goto failed; > } > > > -- > 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] jbd2: free bh when descriptor block checksum fails 2014-09-14 19:13 ` Eric Sandeen @ 2014-09-16 18:43 ` Theodore Ts'o 0 siblings, 0 replies; 6+ messages in thread From: Theodore Ts'o @ 2014-09-16 18:43 UTC (permalink / raw) To: Eric Sandeen; +Cc: Darrick J. Wong, linux-ext4 On Sun, Sep 14, 2014 at 02:13:43PM -0500, Eric Sandeen wrote: > On 9/14/14 12:33 PM, Darrick J. Wong wrote: > > Free the buffer head if the journal descriptor block fails checksum > > verification. > > > > This is the jbd2 port of the e2fsprogs patch "e2fsck: free bh on csum > > verify error in do_one_pass". > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Reviewed-by: Eric Sandeen <sandeen@redhat.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-16 18:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-14 17:32 [PATCH 0/2] ext4/jbd2: misc 3.17 bugfixes, part 2 Darrick J. Wong 2014-09-14 17:32 ` [PATCH 1/2] ext4: check EA value offset when loading Darrick J. Wong 2014-09-16 18:41 ` Theodore Ts'o 2014-09-14 17:33 ` [PATCH 2/2] jbd2: free bh when descriptor block checksum fails Darrick J. Wong 2014-09-14 19:13 ` Eric Sandeen 2014-09-16 18:43 ` Theodore Ts'o
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.