Page MenuHomeFreeBSD

Fix race condition in linuxkpi workqueue
ClosedPublic

Authored by rstone on Fri, Jan 29, 8:32 PM.

Details

Summary

Consider the following scenario:

  1. A delayed_work struct in the WORK_ST_TIMER state.
  2. Thread A calls mod_delayed_work()
  3. Thread B (a callout thread) simultaneously calls

linux_delayed_work_timer_fn()

The following sequence of events is possible:

A: Call linux_cancel_delayed_work()
A: Change state from TIMER TO CANCEL
B: Change state from CANCEL to TASK
B: taskqueue_enqueue() the task
A: taskqueue_cancel() the task
A: Call linux_queue_delayed_work_on(). This is a no-op because the
state is WORK_ST_TASK.

As a result, the delayed_work struct will never be invoked. This is
causing address resolution in ib_addr.c to stop permanently, as it
never tries to reschedule a task that it thinks is already scheduled.

Fix this by introducing locking into the cancel path (which
corresponds with the lock held while the callout runs). This will
prevent the callout from changing the state of the task until the
cancel is complete, preventing the race.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I will test and review this patch on Monday.

sys/compat/linuxkpi/common/src/linux_work.c
495–496

Missing mtx_unlock() before this return statement.

Fix lock leak. Sorry, this review was based off of an
older version of my patch that didn't have that fixed.
I've confirmed that this is now in sync with what was
tested internally at $WORK

Looks good. Don't forget to MFC to 11,12,13.

--HPS

This revision is now accepted and ready to land.Tue, Feb 2, 2:38 PM
This revision was automatically updated to reflect the committed changes.