linux-kernel - Re: [PATCH v1 1/1] xarray: fix the data-race in xas_find_chunk() by using READ

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230918141722.gasntomhkkp2fwy2@quack3>
Date:   Mon, 18 Sep 2023 16:17:22 +0200
From:   Jan Kara <jack@...e.cz>
To:     Mirsad Todorovac <mirsad.todorovac@....unizg.hr>
Cc:     Jan Kara <jack@...e.cz>, Philipp Stanner <pstanner@...hat.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Matthew Wilcox <willy@...radead.org>, Chris Mason <clm@...com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        linux-mm@...ck.org, Yury Norov <yury.norov@...il.com>
Subject: Re: [PATCH v1 1/1] xarray: fix the data-race in xas_find_chunk() by
 using READ_ONCE()

On Mon 18-09-23 15:34:46, Mirsad Todorovac wrote:
> On 9/18/23 15:18, Jan Kara wrote:
> > On Mon 18-09-23 14:46:02, Mirsad Todorovac wrote:
> > > Hi,
> > > 
> > > I tried this patch and the
> > > 
> > > > >    95 _find_first_and_bit (lib/find_bit.c:114 (discriminator 10))
> > > > >    31 _find_first_zero_bit (lib/find_bit.c:125 (discriminator 10))
> > > > > 173 _find_next_and_bit (lib/find_bit.c:171 (discriminator 2))
> > > > > 655 _find_next_bit (lib/find_bit.c:133 (discriminator 2))
> > > > >     5 _find_next_zero_bit
> > > 
> > > data-races do not seem to appear any longer.
> > 
> > Yup. You've just missed one case in _find_last_bit() and then all the
> > functions in include/linux/find.h need a similar treatment...
> 
> I seem to have this:
> 
> -----------------------------------------------------------------------------------
> #ifndef find_last_bit
> unsigned long _find_last_bit(const unsigned long *addr, unsigned long size)
> {
>         if (size) {
>                 unsigned long val = BITMAP_LAST_WORD_MASK(size);
>                 unsigned long idx = (size-1) / BITS_PER_LONG;
> 
>                 do {
>                         val &= READ_ONCE(addr[idx]);
>                         if (val)
>                                 return idx * BITS_PER_LONG + __fls(val);
> 
>                         val = ~0ul;
>                 } while (idx--);
>         }
>         return size;
> }
> EXPORT_SYMBOL(_find_last_bit);
> #endif
> -----------------------------------------------------------------------------------
> 
> Is there something I did not notice?

No, this looks correct. I just somehow didn't see this hunk in the diff
you've posted.

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