| Message ID | 20180613045205.19240-1-tytso@mit.edu |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/2] ext4: add corruption check in ext4_xattr_set_entry() | expand |
On Jun 12, 2018, at 10:52 PM, Theodore Ts'o <tytso@mit.edu> wrote: > > In theory this should have been caught earlier when the xattr list was > verified, but in case it got missed, it's simple enough to add check > to make sure we don't overrun the xattr buffer. > > https://bugzilla.kernel.org/show_bug.cgi?id=200001 > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > fs/ext4/xattr.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index fc4ced59c565..230ba79715f6 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -1560,7 +1560,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i, > handle_t *handle, struct inode *inode, > bool is_block) > { > - struct ext4_xattr_entry *last; > + struct ext4_xattr_entry *last, *next; > struct ext4_xattr_entry *here = s->here; > size_t min_offs = s->end - s->base, name_len = strlen(i->name); > int in_inode = i->in_inode; > @@ -1595,7 +1595,13 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i, > > /* Compute min_offs and last. */ > last = s->first; > - for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) { > + for (; !IS_LAST_ENTRY(last); last = next) { > + next = EXT4_XATTR_NEXT(last); > + if ((void *)next >= s->end) { > + EXT4_ERROR_INODE(inode, "corrupted xattr entries"); > + ret = -EFSCORRUPTED; > + goto out; > + } > if (!last->e_value_inum && last->e_value_size) { > size_t offs = le16_to_cpu(last->e_value_offs); > if (offs < min_offs) > -- > 2.18.0.rc0 > Cheers, Andreas
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index fc4ced59c565..230ba79715f6 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -1560,7 +1560,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i, handle_t *handle, struct inode *inode, bool is_block) { - struct ext4_xattr_entry *last; + struct ext4_xattr_entry *last, *next; struct ext4_xattr_entry *here = s->here; size_t min_offs = s->end - s->base, name_len = strlen(i->name); int in_inode = i->in_inode; @@ -1595,7 +1595,13 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i, /* Compute min_offs and last. */ last = s->first; - for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) { + for (; !IS_LAST_ENTRY(last); last = next) { + next = EXT4_XATTR_NEXT(last); + if ((void *)next >= s->end) { + EXT4_ERROR_INODE(inode, "corrupted xattr entries"); + ret = -EFSCORRUPTED; + goto out; + } if (!last->e_value_inum && last->e_value_size) { size_t offs = le16_to_cpu(last->e_value_offs); if (offs < min_offs)
In theory this should have been caught earlier when the xattr list was verified, but in case it got missed, it's simple enough to add check to make sure we don't overrun the xattr buffer. https://bugzilla.kernel.org/show_bug.cgi?id=200001 Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- fs/ext4/xattr.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)