linux-ext4 - Re: [PATCH] ext4: remove BUG_ON which will be triggered in race scenario

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 13 Apr 2023 10:19:00 +0800
From:   JunChao Sun <sunjunchao2870@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     linux-ext4@...r.kernel.org, tytso@....edu, yi.zhang@...wei.com,
        sunjunchao <sunjunchao@...rongyun.com>
Subject: Re: [PATCH] ext4: remove BUG_ON which will be triggered in race scenario

Jan Kara <jack@...e.cz> 于2023年4月12日周三 18:45写道：
>
> On Wed 12-04-23 00:47:37, JunChao Sun wrote:
> > From: sunjunchao <sunjunchao@...rongyun.com>
> >
> > There is a BUG_ON statement which will be triggered in the
> > following scenario, let's remove it.
> >
> > thread0                                         thread1
> > ext4_write_begin(inode0)
> >     ->ext4_try_to_write_inline_data()
> >         written some bits successfully
> > ext4_write_end(inode0)
> >     ->ext4_write_inline_data_end()
> >                                             ext4_write_begin(inode0)
> >                                                 ->ext4_try_to_write_inline_data()
> >                                                     ->ext4_convert_inline_data_to_extent()
> >                                                         ->ext4_write_lock_xattr()
> >                                                             ->ext4_destroy_inline_data_nolock()
> >                                                                 ->ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> >                                                         ->ext4_write_unlock_xattr()
> >         ->ext4_write_lock_xattr()
> >         ->BUG_ON(!ext4_has_inline_data()) will be triggered
> >
> > The problematic logic is that ext4_write_end() test ext4_has_inline_data()
> > without holding xattr_sem, and ext4_write_inline_data_end() test it again using
> > a BUG_ON() with holding xattr_sem.
>
>
> > Were you able to actually hit this? Because inode->i_rwsem should be
> > protecting us from races like this so I don't think the above described
> > scenario can happen.

Yes, you are right. The write_begin() and write_end() were protected
by inode->i_rwsem. And then I checked the operations to
EXT4_INODE_INLINE_DATA, but didn't find any race. I didn't hit that, I
was reading code and recognized that there may be a race, but wasn't
aware of the inode->i_rwsem due to my carelessness. Sorry for that.

>
>                                                                 Honza
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR