On Wed 03-05-23 22:41:57, Theodore Ts'o wrote: > On Wed, May 03, 2023 at 08:21:28PM +0200, Jan Kara wrote: > > When using cached extent stored in extent status tree in tree->cache_es > > another process holding ei->i_es_lock for reading can be racing with us > > setting new value of tree->cache_es. If the compiler would decide to > > refetch tree->cache_es at an unfortunate moment, it could result in a > > bogus in_range() check. Fix the possible race by using READ_ONCE() when > > using tree->cache_es only under ei->i_es_lock for reading. > > > > Reported-by: syzbot+4a03518df1e31b537066@xxxxxxxxxxxxxxxxxxxxxxxxx > > Link: https://lore.kernel.org/all/000000000000d3b33905fa0fd4a6@xxxxxxxxxx > > Suggested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > Don't we also need a WRITE_ONCE here? Right, we should do that as well. I'll update the patch. Honza > > diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c > index 7bc221038c6c..4694582cf255 100644 > --- a/fs/ext4/extents_status.c > +++ b/fs/ext4/extents_status.c > @@ -293,7 +293,7 @@ static void __es_find_extent_range(struct inode *inode, > } > > if (es1 && matching_fn(es1)) { > - tree->cache_es = es1; > + WRITE_ONCE(tree->cache_es, es1); > es->es_lblk = es1->es_lblk; > es->es_len = es1->es_len; > es->es_pblk = es1->es_pblk; > > - Ted -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
