| Message ID | 20231011150252.32737-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lib/find: Fix KCSAN warnings in find_*_bit() functions | expand |
On Wed, Oct 11, 2023 at 05:02:32PM +0200, Jan Kara wrote: > xas_find_chunk() can be called only under RCU protection and thus tags > can be changing while it is working. Hence the code: > > unsigned long data = *addr & (~0UL << offset); > if (data) > return __ffs(data); > > is prone to 'data' being refetched from addr by the compiler and thus > calling __ffs() with 0 argument and undefined results. > > Fix the problem by removing XA_CHUNK_SIZE == BITS_PER_LONG special case > completely. find_next_bit() gets this right and there's no performance > difference because it is inline and has the very same special branch for > const-sized bitmaps anyway. > > Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> > CC: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
On 10/11/23 17:02, Jan Kara wrote: > xas_find_chunk() can be called only under RCU protection and thus tags > can be changing while it is working. Hence the code: > > unsigned long data = *addr & (~0UL << offset); > if (data) > return __ffs(data); > > is prone to 'data' being refetched from addr by the compiler and thus > calling __ffs() with 0 argument and undefined results. > > Fix the problem by removing XA_CHUNK_SIZE == BITS_PER_LONG special case > completely. find_next_bit() gets this right and there's no performance > difference because it is inline and has the very same special branch for > const-sized bitmaps anyway. > > Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> > CC: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Jan Kara <jack@suse.cz> > --- > include/linux/xarray.h | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/include/linux/xarray.h b/include/linux/xarray.h > index cb571dfcf4b1..07700a2c8870 100644 > --- a/include/linux/xarray.h > +++ b/include/linux/xarray.h > @@ -1718,15 +1718,6 @@ static inline unsigned int xas_find_chunk(struct xa_state *xas, bool advance, > > if (advance) > offset++; > - if (XA_CHUNK_SIZE == BITS_PER_LONG) { > - if (offset < XA_CHUNK_SIZE) { > - unsigned long data = *addr & (~0UL << offset); > - if (data) > - return __ffs(data); > - } > - return XA_CHUNK_SIZE; > - } > - > return find_next_bit(addr, XA_CHUNK_SIZE, offset); > } > Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> Best regards, Mirsad
diff --git a/include/linux/xarray.h b/include/linux/xarray.h index cb571dfcf4b1..07700a2c8870 100644 --- a/include/linux/xarray.h +++ b/include/linux/xarray.h @@ -1718,15 +1718,6 @@ static inline unsigned int xas_find_chunk(struct xa_state *xas, bool advance, if (advance) offset++; - if (XA_CHUNK_SIZE == BITS_PER_LONG) { - if (offset < XA_CHUNK_SIZE) { - unsigned long data = *addr & (~0UL << offset); - if (data) - return __ffs(data); - } - return XA_CHUNK_SIZE; - } - return find_next_bit(addr, XA_CHUNK_SIZE, offset); }
xas_find_chunk() can be called only under RCU protection and thus tags can be changing while it is working. Hence the code: unsigned long data = *addr & (~0UL << offset); if (data) return __ffs(data); is prone to 'data' being refetched from addr by the compiler and thus calling __ffs() with 0 argument and undefined results. Fix the problem by removing XA_CHUNK_SIZE == BITS_PER_LONG special case completely. find_next_bit() gets this right and there's no performance difference because it is inline and has the very same special branch for const-sized bitmaps anyway. Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> CC: Matthew Wilcox <willy@infradead.org> Signed-off-by: Jan Kara <jack@suse.cz> --- include/linux/xarray.h | 9 --------- 1 file changed, 9 deletions(-)