Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refuse to destroy a jail with mounted filesystems #667

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gahr
Copy link

@gahr gahr commented Jan 22, 2024

# bastille create temp 14.0-RELEASE 192.168.1.10 lo1
...
# mkdir test
# cp /usr/bin/less test/
# mkdir /usr/local/bastille/jails/temp/root/test
# bastille mount temp $(realpath test) test
[temp]:
Added: /root/admin/bastille/test /usr/local/bastille/jails/temp/root/test nullfs ro 0 0
# /usr/local/bastille/jails/temp/root/test/less -f /dev/stdin &
# bastille destroy force temp
rdr-anchor not found in pf.conf
[temp]:
temp: removed
umount: unmount of /usr/local/bastille/jails/temp/root/test failed: Device busy
jail: temp: /sbin/umount -t nullfs /usr/local/bastille/jails/temp/root/test: failed

Deleting Jail: temp.
Jail has mounted filesystems:
/usr/local/bastille/jails/temp/root/test

Fixes #662

@gahr gahr force-pushed the refuse-to-destroy-mounted branch 2 times, most recently from b56ec46 to 54e3cd2 Compare January 22, 2024 09:28
```
 # bastille create temp 14.0-RELEASE 192.168.1.10 lo1
 ...
 # mkdir test
 # cp /usr/bin/less test/
 # mkdir /usr/local/bastille/jails/temp/root/test
 # bastille mount temp $(realpath test) test
 [temp]:
 Added: /root/admin/bastille/test
 /usr/local/bastille/jails/temp/root/test nullfs ro 0 0
 # /usr/local/bastille/jails/temp/root/test/less -f /dev/stdin &
 # bastille destroy force temp
 rdr-anchor not found in pf.conf
 [temp]:
 temp: removed
 umount: unmount of /usr/local/bastille/jails/temp/root/test failed:
 Device busy
 jail: temp: /sbin/umount -t nullfs
 /usr/local/bastille/jails/temp/root/test: failed

 Deleting Jail: temp.
 Jail has mounted filesystems:
 /usr/local/bastille/jails/temp/root/test
```
web-github pushed a commit to SuperScript/bastille that referenced this pull request Apr 24, 2024
@yaazkal yaazkal added the enhancement New feature or request label Jul 8, 2024
@web-sst
Copy link
Contributor

web-sst commented Jul 9, 2024

I think this should be considered a bug fix rather than an enhancement. A likely consequence of operating without this protection is the loss of files within the mounted file system. That's a very bad outcome and not something a user would expect or think to guard against externally.

@yaazkal
Copy link
Collaborator

yaazkal commented Jul 14, 2024

@gahr in your PR, the message comes after the dataset has been destroyed if it's on ZFS. If we are going to have that warning, it should come earlier so the user can check mounts and try to destroy again.

@gahr
Copy link
Author

gahr commented Jul 15, 2024

Good point. I don't use ZFS myself so I can't easily test any changes in that area. Would you please improve my PR in that direction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] destroy removes potentially mounted directories
3 participants