bwrap: Bind-mount host /dev instead of creating fresh devtmpfs#2048
bwrap: Bind-mount host /dev instead of creating fresh devtmpfs#2048ckyrouac merged 1 commit intobootc-dev:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request changes how bwrap sets up the /dev directory in the sandbox, switching from a minimal devtmpfs to a bind mount of the host's /dev. While this change aims to improve device visibility and allow tools like lsblk to properly enumerate partition children, it significantly weakens the security isolation by granting full access to all host devices inside the sandbox, which is a regression in the security posture. Additionally, there is a suggestion to improve error handling when checking for the existence of /run/udev to make it more robust.
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Gemini had a nit, but we can also fix it after if you want as part of other things
Hmm that's concerning that looks like we're leaking content into the target deployment root. Ah yes I think we're missing |
crates/utils/src/bwrap.rs
Outdated
| // can read partition type GUIDs and other device properties. | ||
| if Utf8Path::new("/run/udev").try_exists()? { | ||
| cmd.args(["--bind", "/run/udev", "/run/udev"]); | ||
| cmd.args(["--tmpfs", "/run"]); |
There was a problem hiding this comment.
Actually on reflection I think it's probably better to just --bind /run /run - there's just too many things there that we may want; same as what we do when in podman.
(But even if we didn't take that path, we would need to always pass the --tmpfs /run unconditionally)
There was a problem hiding this comment.
good idea - updated
Replace `--dev /dev` with `--dev-bind /dev /dev` in the bwrap container setup so that lsblk inside the sandbox can properly enumerate partition children of block devices (e.g. loop devices). The previous approach created a minimal devtmpfs that lacked complete device information, causing ESP partition discovery to fail inside the bwrap sandbox. With a full bind-mount of host /dev, the per-device bind_device() mechanism is no longer needed and is removed. Additionally, bind-mount /run/udev into the sandbox when it exists so that lsblk and libblkid can read the udev database for partition type GUIDs and other device properties. Without this, tools that query device metadata (e.g. PARTTYPE) would get incomplete results even with /dev properly mounted. Assisted-by: Claude Code (Opus 4) Signed-off-by: ckyrouac <ckyrouac@redhat.com>
2 participants
Replace
--dev /devwith--dev-bind /dev /devin the bwrap container setup so that lsblk inside the sandbox can properly enumerate partition children of block devices (e.g. loop devices).The previous approach created a minimal devtmpfs that lacked complete device information, causing ESP partition discovery to fail inside the bwrap sandbox. With a full bind-mount of host /dev, the per-device bind_device() mechanism is no longer needed and is removed.
Additionally, bind-mount /run/udev into the sandbox when it exists so that lsblk and libblkid can read the udev database for partition type GUIDs and other device properties. Without this, tools that query device metadata (e.g. PARTTYPE) would get incomplete results even with /dev properly mounted.
Assisted-by: Claude Code (Opus 4)