From: Jann Horn Date: Wed, 22 Aug 2018 04:59:37 +0000 (-0700) Subject: reiserfs: fix broken xattr handling (heap corruption, bad retval) X-Git-Tag: v3.18.120~1 X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88f8dcdb69e35da54b5b7579403d365df457f093;p=linux reiserfs: fix broken xattr handling (heap corruption, bad retval) commit a13f085d111e90469faf2d9965eb39b11c114d7e upstream. This fixes the following issues: - When a buffer size is supplied to reiserfs_listxattr() such that each individual name fits, but the concatenation of all names doesn't fit, reiserfs_listxattr() overflows the supplied buffer. This leads to a kernel heap overflow (verified using KASAN) followed by an out-of-bounds usercopy and is therefore a security bug. - When a buffer size is supplied to reiserfs_listxattr() such that a name doesn't fit, -ERANGE should be returned. But reiserfs instead just truncates the list of names; I have verified that if the only xattr on a file has a longer name than the supplied buffer length, listxattr() incorrectly returns zero. With my patch applied, -ERANGE is returned in both cases and the memory corruption doesn't happen anymore. Credit for making me clean this code up a bit goes to Al Viro, who pointed out that the ->actor calling convention is suboptimal and should be changed. Link: http://lkml.kernel.org/r/20180802151539.5373-1-jannh@google.com Fixes: 48b32a3553a5 ("reiserfs: use generic xattr handlers") Signed-off-by: Jann Horn Acked-by: Jeff Mahoney Cc: Eric Biggers Cc: Al Viro Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman --- diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index 7c36898af402..59b29acb6419 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c @@ -842,8 +842,10 @@ static int listxattr_filler(void *buf, const char *name, int namelen, size = handler->list(b->dentry, b->buf + b->pos, b->size, name, namelen, handler->flags); - if (size > b->size) + if (b->pos + size > b->size) { + b->pos = -ERANGE; return -ERANGE; + } } else { size = handler->list(b->dentry, NULL, 0, name, namelen, handler->flags);