On 3/24/21 10:47 AM, Tetsuo Handa wrote: > On 2021/03/23 0:31, Jan Kara wrote: >> Thanks for the patch Tetsuo! I'd prefer if Jeff had a look since he has >> written this code back then. But let me provide my view: I agree that for a >> corrupted filesystem it can happen that xattr_root remains NULL although >> priv_root is set. So your change makes sense. But then >> reiserfs_xattrs_initialized() seems to be used really minimally? Only once >> in fs/reiserfs/xattr_security.c and e.g. reiserfs_xattr_set() is prone to >> the same problem? Do I miss something? > > As far as tested with assertion patch > ( https://syzkaller.appspot.com/text?tag=Patch&x=13186fe6d00000 ) applied, > syzbot did not trigger the BUG_ON() added by this patch, which means that > reiserfs_fill_super() always fails if reiserfs_xattrs_initialized() returned false. > > And console log ( https://syzkaller.appspot.com/text?tag=CrashLog&x=177b30bad00000 ) contains > > jdm-20006 create_privroot: xattrs/ACLs enabled and couldn't find/create .reiserfs_priv. Failing mount. > > messages, which means that e.g. reiserfs_xattr_set() will not be called on > this corrupted filesystem image because mount operation itself fails. > > Despite there are other bugs remaining, I think that applying this patch as-is is OK. > Ever wish you had a time machine and could go back and prevent your former self from making a mistake? The reiserfs xattr code would be my first destination. Tetsuo's patch is fine but it needs a similar fix in reiserfs_xattr_set, as you noted. Whether it's required is another question. ReiserFS is absolutely loaded with fuzzer bugs. -Jeff -- Jeff Mahoney Director, SUSE Labs Data & Performance
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
