swapoff: don't leak vnode pointer to device

[brooks]

Summary

After looking up the device path, we compare the vnode to the vnode of
each swap device. Prior to this change we leaked a vnode reference to
the device we're trying to remove from the swap pool.

Reported by: llm-postcr
MFC after: 3 days
Sponsored by: Innovate UK

[brooks]

postcr report:

POSTCR-965170da4548176cb0ec4ee65e01248af37f8824-1: pre-existing: kern_swapoff()               
never vrele()s the vnode returned by namei(), leaking a usecount on every call                
where the lookup succeeds                                                                     
                                                                                              
kern_swapoff() calls namei() without LOCKLEAF. It uses nd.ni_vp only to compare               
the pointer against sp->sw_vp and never drops that reference. The leak happens                
on every return after a successful namei():                                                   
- the EINVAL "not a swap device" return                                                       
- the ENOMEM and MAC-denial returns in swapoff_one()                                          
- the success path                                                                            
                                                                                              
sw_vp holds its own reference from swapon, so the lookup's reference is simply                
lost.                                                                                         
                                                                                              
Example: "swapoff /mnt/nfs/swapfile" succeeds, but a later plain "umount                      
/mnt/nfs" fails with EBUSY because vflush() without FORCECLOSE still sees the                 
usecount. Running swapoff on any regular file has the same effect on that file's              
filesystem.                                                                                   
                                                                                              
Origin: the bug dates from the original swapoff code, commit 92da00bb245b                     
(2002). At that time the success path did one vrele(), but it balanced the                    
swapon reference, not the lookup's. That vrele() later moved into the                         
per-backend sw_close method in commit dee34ca4fcf0 (2003), which added that                   
method.                                                                                       
                                                                                              
Suggested fix: add vrele(vp) right after the TAILQ_FOREACH lookup. The reference              
held through sw_vp keeps the vnode alive for swapoff_one().

[markj]

sys/vm/swap_pager.c

2830

Shouldn't the rele be done here in swapoff_one()?

I don't think we can call vrele() under a mutex, if the usecount transitions 1->0 we might acquire the (sleepable) vnode lock.

[brooks]

Host the vrele out of the swap device lock region

[brooks]

sys/vm/swap_pager.c

2830

Shouldn't the rele be done here in swapoff_one()?

vp is some potentially arbitrary path's vnode not necessarily the swap device so we need to release that reference here. D61465 has a second bug about the swap device's vnode not being released.

I don't think we can call vrele() under a mutex, if the usecount transitions 1->0 we might acquire the (sleepable) vnode lock.

This is true. I tested on an arbitrary device (/dev/fd) and got a LOR.