Re: [PATCH] reiserfs: fix broken xattr handling (heap corruption, bad retval)

[Will Deacon]

Hi Jann,

On Fri, Aug 10, 2018 at 05:19:38AM +0200, Jann Horn wrote:
> On Thu, Aug 2, 2018 at 5:16 PM Jann Horn <jannh@xxxxxxxxxx> wrote:
> >
> > 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.
> >
> > Fixes: 48b32a3553a5 ("reiserfs: use generic xattr handlers")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
> 
> +security@
> Ping. I have not received any replies to this patch, which fixes a
> kernel security bug, for a week.
> Whose tree should this go through? reiserfs is marked as "supported",
> but does not have a maintainer or a git repo listed, just a
> mailinglist, so I guess it probably has to go through either Al Viro's
> or akpm's tree? Looks like akpm signed off on the last commits in
> reiserfs...

I think Andrew's tree makes the most sense for this, but perhaps we should
also patch MAINTAINERS so mark it as "Orphan"? Patch below.

Will

--->8