Skip to content

build-rootfs.sh: cleanup trap can fail to unmount when chroot install errored mid-way #264

Description

@hazelp343

scripts/build-rootfs.sh mounts /proc, /sys, /dev for chroot, then runs apt inside the chroot. The cleanup is:

cleanup() {
    sudo umount "$WORK/dev"  2>/dev/null || true
    sudo umount "$WORK/sys"  2>/dev/null || true
    sudo umount "$WORK/proc" 2>/dev/null || true
    docker rm -f "$CONTAINER" >/dev/null 2>&1 || true
    sudo rm -rf "$WORK" 2>/dev/null || true
}
trap cleanup EXIT

If the chroot apt install crashes badly enough that kernel processes hold open files in $WORK/proc or $WORK/dev, the umount returns EBUSY. The script silently swallows that, then sudo rm -rf "$WORK" proceeds — and recursively descends into the still-mounted /dev, potentially deleting host device nodes if the mount is misbehaving. (In practice --bind mounts don't follow into the host, but the safety margin is thin.)

Better cleanup:

cleanup() {
    # Force lazy unmount so even busy mounts come down without taking us with them
    for mnt in dev sys proc; do
        sudo umount -l "$WORK/$mnt" 2>/dev/null || true
    done
    docker rm -f "$CONTAINER" >/dev/null 2>&1 || true
    # Guard: refuse to rm -rf if anything is still mounted under $WORK
    if mount | grep -q "$WORK"; then
        echo "warning: refusing to rm $WORK because mounts remain"
        return
    fi
    sudo rm -rf "$WORK"
}

The mount | grep check is the safety net that ensures we don't rm -rf over a still-bind-mounted host directory.

Severity: Low (current code probably has been fine in practice), but the design has too thin a safety margin for a sudo'd recursive remove.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions