Hello,
Our static analysis tool finds some possible data races in the OCFS2 file
system in Linux 6.4.0-rc6.
In most calling contexts, the variables such as res->lockname.name and
res->owner are accessed with holding the lock res->spinlock. Here is an
example:
lockres_seq_start() --> Line 539 in dlmdebug.c
spin_lock(&res->spinlock); --> Line 574 in dlmdebug.c (Lock
res->spinlock)
dump_lockres(res, ...); --> Line 575 in fs/ocfs2/dlm/dlmdebug.c
stringify_lockname(res->lockname.name, ...); --> Line 493 in
dlmdebug.c (Access res->lockname.name)
scnprintf(..., res->owner, ...); -->Line 498 in dlmdebug.c
(Access res->owner)
However, in the following calling contexts:
dlm_deref_lockres_worker() --> Line 2439 in dlmmaster.c
dlm_drop_lockres_ref_done() --> Line 2459 in dlmmaster.c
lockname = res->lockname.name; --> Line 2416 in dlmmaster.c
(Access res->lockname.name)
dlm_get_lock_resource() --> Line 701 in dlmmaster.c
if (res->owner != dlm->node_num) --> Line 1023 in dlmmaster.c
(Access res->owner)
The variables res->lockname.name and res->owner are accessed respectively
without holding the lock res->spinlock, and thus data races can occur.
I am not quite sure whether these possible data races are real and how
to fix
them if they are real.
Any feedback would be appreciated, thanks!
Reported-by: BassCheck <[email protected]>
Best wishes,
Tuo Li
Hi,
On 6/13/23 4:23 PM, Tuo Li wrote:
> Hello,
>
> Our static analysis tool finds some possible data races in the OCFS2 file
> system in Linux 6.4.0-rc6.
>
> In most calling contexts, the variables such as res->lockname.name and
> res->owner are accessed with holding the lock res->spinlock. Here is an
> example:
>
> lockres_seq_start() --> Line 539 in dlmdebug.c
> spin_lock(&res->spinlock); --> Line 574 in dlmdebug.c (Lock
> res->spinlock)
> dump_lockres(res, ...); --> Line 575 in fs/ocfs2/dlm/dlmdebug.c
> stringify_lockname(res->lockname.name, ...); --> Line 493 in
> dlmdebug.c (Access res->lockname.name)
> scnprintf(..., res->owner, ...); -->Line 498 in dlmdebug.c (Access
> res->owner)
>
> However, in the following calling contexts:
>
> dlm_deref_lockres_worker() --> Line 2439 in dlmmaster.c
> dlm_drop_lockres_ref_done() --> Line 2459 in dlmmaster.c
> lockname = res->lockname.name; --> Line 2416 in dlmmaster.c (Access
> res->lockname.name)
lockname won't changed during the lockres lifecycle.
So this won't cause any real problem since now it holds a reference.
>
> dlm_get_lock_resource() --> Line 701 in dlmmaster.c
> if (res->owner != dlm->node_num) --> Line 1023 in dlmmaster.c (Access
> res->owner)
Do you mean in dlm_wait_for_lock_mastery()?
Even if owner changes suddenly, it will recheck, so I think it is also fine.
Thanks,
Joseph
>
> The variables res->lockname.name and res->owner are accessed respectively
> without holding the lock res->spinlock, and thus data races can occur.
>
> I am not quite sure whether these possible data races are real and how to
> fix
> them if they are real.
>
> Any feedback would be appreciated, thanks!
>
> Reported-by: BassCheck <[email protected]> <[email protected]>
>
> Best wishes,
> Tuo Li
>