| From a13f085d111e90469faf2d9965eb39b11c114d7e Mon Sep 17 00:00:00 2001 |
| From: Jann Horn <jannh@google.com> |
| Date: Tue, 21 Aug 2018 21:59:37 -0700 |
| Subject: reiserfs: fix broken xattr handling (heap corruption, bad retval) |
| |
| From: Jann Horn <jannh@google.com> |
| |
| 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 <jannh@google.com> |
| Acked-by: Jeff Mahoney <jeffm@suse.com> |
| Cc: Eric Biggers <ebiggers@google.com> |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Cc: <stable@vger.kernel.org> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/reiserfs/xattr.c | 4 +++- |
| 1 file changed, 3 insertions(+), 1 deletion(-) |
| |
| --- a/fs/reiserfs/xattr.c |
| +++ b/fs/reiserfs/xattr.c |
| @@ -791,8 +791,10 @@ static int listxattr_filler(struct dir_c |
| return 0; |
| size = namelen + 1; |
| if (b->buf) { |
| - if (size > b->size) |
| + if (b->pos + size > b->size) { |
| + b->pos = -ERANGE; |
| return -ERANGE; |
| + } |
| memcpy(b->buf + b->pos, name, namelen); |
| b->buf[b->pos + namelen] = 0; |
| } |