Page MenuHomeFreeBSD

Fix a swap block allocation race.
ClosedPublic

Authored by markj on Thu, Feb 13, 5:24 PM.

Details

Summary

putpages' allocation of swap blocks is done under the global sw_dev
lock. Previously it would drop that lock before inserting the allocated
blocks into the object's trie, creating a window in which swap blocks
are allocated but are not visible to swapoff. This can cause
swp_pager_strategy() to fail and panic the system.

Fix the problem bluntly, by allocating swap blocks under the object
lock.

Diff Detail

Repository
rS 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

markj created this revision.Thu, Feb 13, 5:24 PM
markj updated this revision to Diff 68263.Thu, Feb 13, 6:28 PM

Wake up sleepers when incrementing nsw_wcount_async.

jeff accepted this revision.Thu, Feb 13, 11:03 PM
jeff added inline comments.
sys/vm/swap_pager.c
1457 ↗(On Diff #68263)

We should generalize my lockless limit/counting semaphore in uma.

1482–1483 ↗(On Diff #68263)

Unrelated but it's unfortunate that this is required. How many places actually inspect that we can't pass NULL?

1502 ↗(On Diff #68263)

Wouldn't need the double loop if we allocated the buf first. Why did it need to move?

This revision is now accepted and ready to land.Thu, Feb 13, 11:03 PM
markj added inline comments.Fri, Feb 14, 3:35 PM
sys/vm/swap_pager.c
1482–1483 ↗(On Diff #68263)

I'm actually not sure that this is really required, though at least nfs_strategy() assumes that the creds are not equal to NOCRED (NFSNEWCRED will trigger a page fault), and yes it is in principle possible to swap over NFS. The buffer cache seems to handle null credentials. I don't see why we bother setting b_rcred though.

1502 ↗(On Diff #68263)

It doesn't really need to move. It is just more cleanup to do if the swap block allocation fails and I liked having all of the initialization happen in one block. Maybe I will change it to avoid the extra loop.

kib accepted this revision.Sat, Feb 15, 7:51 PM
This revision was automatically updated to reflect the committed changes.