#1485 closed defect

Locks while accessing sync_active field in bdbuf library

Reported by: Oleg Owned by: Sebastian Huber
Priority: normal Milestone: 4.10.3
Component: lib/block Version: 4.10
Severity: normal Keywords:
Cc: thomas.doerfler@…, chrisj@…, nbkolchin@…, sebastian.huber@… Blocked By:
Blocking:

Description (last modified by Gedare Bloom)

In rtems_bdbuf_swapout_processing() function there is the following lines:

if (bdbuf_cache.sync_active && !transfered_buffers)
{

rtems_id sync_requester;
rtems_bdbuf_lock_cache ();
...

}

Here access to bdbuf_cache.sync_active is not protected with anything.
Imagine the following test case:

  1. Task1 releases buffer(s) with bdbuf_release_modified() calls;
  2. After a while swapout task starts and flushes all buffers;
  3. In the end of that swapout flush we are before that part of code, and assume there is task switching (just before "if (bdbuf_cache.sync_active && !transfered_buffers)");
  4. Some other task (with higher priority) does bdbuf_release_modified and rtems_bdbuf_syncdev().

This task successfully gets both locks sync and pool (in rtems_bdbuf_syncdev() function), sets sync_active to true and starts waiting for RTEMS_BDBUF_TRANSFER_SYNC event with only sync lock got.

  1. Task switching happens again and we are again before "if (bdbuf_cache.sync_active && !transfered_buffers)".

As the result we check sync_active and we come inside that "if" statement.

  1. The result is that we send RTEMS_BDBUF_TRANSFER_SYNC event! Though ALL modified messages of that task are not flushed yet!

If that high priority task re-boots a board after rtems_bdbuf_syncdev() return we have a very good bug. Imagine a nuclear power station is running such a code, or some space shuttle :-)

Change History (6)

comment:1 Changed on 12/27/09 at 00:48:59 by Chris Johns

My reading of the code is the sync_access field is protected by the cache lock.

The code you reference is in rtems_bdbuf_swapout_processing and there is a call to rtems_bdbuf_lock_cache at the start of the function. Is this lock being released before the code you reference ?

comment:2 Changed on 11/22/14 at 13:47:52 by Gedare Bloom

Description: modified (diff)
Milestone: 4.104.10.3

comment:3 Changed on 11/23/14 at 16:58:48 by Joel Sherrill

Owner: changed from Joel Sherrill to Sebastian Huber
Status: newassigned

comment:4 Changed on 11/28/14 at 10:13:04 by Sebastian Huber <sebastian.huber@…>

Resolution: fixed
Status: assignedclosed

In 3b4ca3ab0f99d15794a3eee60b5735f834fd898c/rtems:

bdbuf: Fix race condition with sync active flag

Bug report by Oleg Kravtsov:

In rtems_bdbuf_swapout_processing() function there is the following
lines:

if (bdbuf_cache.sync_active && !transfered_buffers)
{

rtems_id sync_requester;
rtems_bdbuf_lock_cache ();
...

}

Here access to bdbuf_cache.sync_active is not protected with anything.
Imagine the following test case:

  1. Task1 releases buffer(s) with bdbuf_release_modified() calls;
  1. After a while swapout task starts and flushes all buffers;
  1. In the end of that swapout flush we are before that part of code, and

assume there is task switching (just before "if (bdbuf_cache.sync_active
&& !transfered_buffers)");

  1. Some other task (with higher priority) does bdbuf_release_modified

and rtems_bdbuf_syncdev().

This task successfully gets both locks sync and pool (in
rtems_bdbuf_syncdev() function), sets sync_active to true and starts
waiting for RTEMS_BDBUF_TRANSFER_SYNC event with only sync lock got.

  1. Task switching happens again and we are again before "if

(bdbuf_cache.sync_active && !transfered_buffers)".

As the result we check sync_active and we come inside that "if"
statement.

  1. The result is that we send RTEMS_BDBUF_TRANSFER_SYNC event! Though

ALL modified messages of that task are not flushed yet!

close #1485

comment:5 Changed on 11/28/14 at 10:16:17 by Sebastian Huber <sebastian.huber@…>

In f0f2a3dc19e8caddccbb756898b5a413dad9de6a/rtems:

bdbuf: Fix race condition with sync active flag

Bug report by Oleg Kravtsov:

In rtems_bdbuf_swapout_processing() function there is the following
lines:

if (bdbuf_cache.sync_active && !transfered_buffers)
{

rtems_id sync_requester;
rtems_bdbuf_lock_cache ();
...

}

Here access to bdbuf_cache.sync_active is not protected with anything.
Imagine the following test case:

  1. Task1 releases buffer(s) with bdbuf_release_modified() calls;
  1. After a while swapout task starts and flushes all buffers;
  1. In the end of that swapout flush we are before that part of code, and

assume there is task switching (just before "if (bdbuf_cache.sync_active
&& !transfered_buffers)");

  1. Some other task (with higher priority) does bdbuf_release_modified

and rtems_bdbuf_syncdev().

This task successfully gets both locks sync and pool (in
rtems_bdbuf_syncdev() function), sets sync_active to true and starts
waiting for RTEMS_BDBUF_TRANSFER_SYNC event with only sync lock got.

  1. Task switching happens again and we are again before "if

(bdbuf_cache.sync_active && !transfered_buffers)".

As the result we check sync_active and we come inside that "if"
statement.

  1. The result is that we send RTEMS_BDBUF_TRANSFER_SYNC event! Though

ALL modified messages of that task are not flushed yet!

close #1485

comment:6 Changed on 10/10/17 at 06:49:19 by Sebastian Huber

Component: fslib/block
Note: See TracTickets for help on using tickets.