[RTEMS Project] #1485: Locks while accessing sync_active field in bdbuf library

[RTEMS trac]

#1485: Locks while accessing sync_active field in bdbuf library
---------------------------+----------------------------
 Reporter:  Oleg.Kravtsov  |       Owner:  joel.sherrill
     Type:  defect         |      Status:  new
 Priority:  normal         |   Milestone:  4.10.3
Component:  filesystem     |     Version:  4.10
 Severity:  normal         |  Resolution:
 Keywords:                 |
---------------------------+----------------------------
Changes (by gedare):

 * milestone:  4.10 => 4.10.3


Old description:

> 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.
> 5. 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.
> 6. 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 :-)

New description:

 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.
 5. 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.
 6. 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 :-)

--

--
Ticket URL: <http://devel.rtems.org/ticket/1485#comment:2>
RTEMS Project <http://www.rtems.org/>
RTEMS Project