Page MenuHomeFreeBSD

Fix a swap block allocation race.
ClosedPublic

Authored by markj on Feb 13 2020, 5:24 PM.
Tags
None
Referenced Files
F80132875: D23665.diff
Thu, Mar 28, 8:39 AM
Unknown Object (File)
Jan 7 2024, 1:14 AM
Unknown Object (File)
Dec 24 2023, 9:17 PM
Unknown Object (File)
Dec 22 2023, 11:11 PM
Unknown Object (File)
Nov 2 2023, 6:06 AM
Unknown Object (File)
Sep 4 2023, 12:44 PM
Unknown Object (File)
Sep 4 2023, 12:43 PM
Unknown Object (File)
Sep 4 2023, 12:43 PM
Subscribers

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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 29351
Build 27250: arc lint + arc unit

Event Timeline

Wake up sleepers when incrementing nsw_wcount_async.

jeff added inline comments.
sys/vm/swap_pager.c
1457

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

1482–1483

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

1501

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.Feb 13 2020, 11:03 PM
sys/vm/swap_pager.c
1482–1483

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.

1501

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.

This revision was automatically updated to reflect the committed changes.