[Ocfs2-devel] [BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Ocfs2-devel] [BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Tuo Li via Ocfs2-devel]

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 <bass@buaa.edu.cn>

Best wishes,
Tuo Li

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

[BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Tuo Li]

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 <bass@buaa.edu.cn>

Best wishes,
Tuo Li

[Ocfs2-devel] [BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Tuo Li via Ocfs2-devel]

[-- Attachment #1.1: Type: text/plain, Size: 1428 bytes --]

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 <bass@buaa.edu.cn> <bass@buaa.edu.cn>

Best wishes,
Tuo Li

[-- Attachment #1.2: Type: text/html, Size: 2033 bytes --]

[-- Attachment #2: Type: text/plain, Size: 151 bytes --]

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: [Ocfs2-devel] [BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Joseph Qi via Ocfs2-devel]

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 <bass@buaa.edu.cn> <bass@buaa.edu.cn>
> 
> Best wishes,
> Tuo Li
> 

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: [Ocfs2-devel] [BUG] ocfs2/dlm: possible data races in dlm_drop_lockres_ref_done() and dlm_get_lock_resource()

[Tuo Li via Ocfs2-devel]

[-- Attachment #1.1: Type: text/plain, Size: 2565 bytes --]

Hello,

Thanks for your reply! It is very helpful!

Do you mean in dlm_wait_for_lock_mastery()?
>

Yes, it is dlm_wait_for_lock_mastery(). I am sorry to confuse you.

Even if owner changes suddenly, it will recheck, so I think it is also
> fine.


Does 'recheck' here mean if owner changes, it will go to the label
'recheck' at Line 1011.
If so, when rechecking, the race can occur again at Line 1023. And thus can
cause infinite rechecking in extreme cases.

Thanks,
Tuo Li

On Fri, Jun 16, 2023 at 10:11 AM Joseph Qi <joseph.qi@linux.alibaba.com>
wrote:

> 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 <bass@buaa.edu.cn> <bass@buaa.edu.cn>
> >
> > Best wishes,
> > Tuo Li
> >
>

[-- Attachment #1.2: Type: text/html, Size: 4182 bytes --]

[-- Attachment #2: Type: text/plain, Size: 151 bytes --]

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel