[Bug 207715] New: xfs: data race on lip->li_lsn in xfs_trans_ail_update_bulk()

[Bug 207715] New: xfs: data race on lip->li_lsn in xfs_trans_ail_update_bulk()

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=207715

            Bug ID: 207715
           Summary: xfs: data race on lip->li_lsn in
                    xfs_trans_ail_update_bulk()
           Product: File System
           Version: 2.5
    Kernel Version: 5.4
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: XFS
          Assignee: filesystem_xfs@kernel-bugs.kernel.org
          Reporter: baijiaju1990@gmail.com
        Regression: No

The functions xfs_trans_ail_update_bulk() and xfs_inode_item_format_core() are
concurrently executed at runtime in the following call contexts:

Thread 1:
xlog_ioend_work()
  xlog_state_done_syncing()
    xlog_state_do_callback()
      xlog_state_do_iclog_callbacks()
        xlog_cil_process_committed()
          xlog_cil_committed()
            xfs_trans_committed_bulk()
              xfs_log_item_batch_insert()
                xfs_trans_ail_update_bulk()

Thread 2:
xfs_file_write_iter()
  xfs_file_buffered_aio_write()
    xfs_file_aio_write_checks()
      xfs_vn_update_time()
        xfs_trans_commit()
          __xfs_trans_commit()
            xfs_log_commit_cil()
              xlog_cil_insert_items()
                xlog_cil_insert_format_items()
                  xfs_inode_item_format()
                    xfs_inode_item_format_core()

In xfs_trans_ail_update_bulk():
  lip->li_lsn = lsn;

In xfs_inode_item_format_core():
  xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);

The variables lip->li_lsn and ip->i_itemp->ili_item.li_lsn access the same
memory, and thus a data race can occur.

This data race was found and actually reproduced by our concurrency fuzzer.

I am not sure whether this data race is harmful and how to fix this data race
properly, so I want to listen to your opinions, thanks :)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Re: [Bug 207715] New: xfs: data race on lip->li_lsn in xfs_trans_ail_update_bulk()

[Dave Chinner]

On Wed, May 13, 2020 at 03:21:05AM +0000, bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=207715
> 
>             Bug ID: 207715
>            Summary: xfs: data race on lip->li_lsn in
>                     xfs_trans_ail_update_bulk()
>            Product: File System
>            Version: 2.5
>     Kernel Version: 5.4
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: XFS
>           Assignee: filesystem_xfs@kernel-bugs.kernel.org
>           Reporter: baijiaju1990@gmail.com
>         Regression: No
> 
> The functions xfs_trans_ail_update_bulk() and xfs_inode_item_format_core() are
> concurrently executed at runtime in the following call contexts:
> 
> Thread 1:
> xlog_ioend_work()
>   xlog_state_done_syncing()
>     xlog_state_do_callback()
>       xlog_state_do_iclog_callbacks()
>         xlog_cil_process_committed()
>           xlog_cil_committed()
>             xfs_trans_committed_bulk()
>               xfs_log_item_batch_insert()
>                 xfs_trans_ail_update_bulk()
> 
> Thread 2:
> xfs_file_write_iter()
>   xfs_file_buffered_aio_write()
>     xfs_file_aio_write_checks()
>       xfs_vn_update_time()
>         xfs_trans_commit()
>           __xfs_trans_commit()
>             xfs_log_commit_cil()
>               xlog_cil_insert_items()
>                 xlog_cil_insert_format_items()
>                   xfs_inode_item_format()
>                     xfs_inode_item_format_core()
> 
> In xfs_trans_ail_update_bulk():
>   lip->li_lsn = lsn;
> 
> In xfs_inode_item_format_core():
>   xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);

Probably a bug on 32 bit systems where a torn write can be seen.
Likely should use xfs_trans_ail_copy_lsn() to sample the lsn out
of the log item.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[Bug 207715] xfs: data race on lip->li_lsn in xfs_trans_ail_update_bulk()

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=207715

--- Comment #1 from Dave Chinner (david@fromorbit.com) ---
On Wed, May 13, 2020 at 03:21:05AM +0000, bugzilla-daemon@bugzilla.kernel.org
wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=207715
> 
>             Bug ID: 207715
>            Summary: xfs: data race on lip->li_lsn in
>                     xfs_trans_ail_update_bulk()
>            Product: File System
>            Version: 2.5
>     Kernel Version: 5.4
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: XFS
>           Assignee: filesystem_xfs@kernel-bugs.kernel.org
>           Reporter: baijiaju1990@gmail.com
>         Regression: No
> 
> The functions xfs_trans_ail_update_bulk() and xfs_inode_item_format_core()
> are
> concurrently executed at runtime in the following call contexts:
> 
> Thread 1:
> xlog_ioend_work()
>   xlog_state_done_syncing()
>     xlog_state_do_callback()
>       xlog_state_do_iclog_callbacks()
>         xlog_cil_process_committed()
>           xlog_cil_committed()
>             xfs_trans_committed_bulk()
>               xfs_log_item_batch_insert()
>                 xfs_trans_ail_update_bulk()
> 
> Thread 2:
> xfs_file_write_iter()
>   xfs_file_buffered_aio_write()
>     xfs_file_aio_write_checks()
>       xfs_vn_update_time()
>         xfs_trans_commit()
>           __xfs_trans_commit()
>             xfs_log_commit_cil()
>               xlog_cil_insert_items()
>                 xlog_cil_insert_format_items()
>                   xfs_inode_item_format()
>                     xfs_inode_item_format_core()
> 
> In xfs_trans_ail_update_bulk():
>   lip->li_lsn = lsn;
> 
> In xfs_inode_item_format_core():
>   xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);

Probably a bug on 32 bit systems where a torn write can be seen.
Likely should use xfs_trans_ail_copy_lsn() to sample the lsn out
of the log item.

Cheers,

Dave.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 207715] xfs: data race on lip->li_lsn in xfs_trans_ail_update_bulk()

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=207715

--- Comment #2 from Jia-Ju Bai (baijiaju1990@gmail.com) ---
Okay, thanks for the reply.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.