Improve error feedback when linux bind mount omits "bind" option?

[mpuncel]

I recently was attempting to modify the standard generated config.json file to add a bind mount to the readonly rootfs for my container.

I added a mount that looked like the below, based on the fact that

$ mount --bind /var/log/foo /path/to/rootfs/var/log/foo

worked the way I wanted it to:

{
        "destination": "/var/log/foo",
        "type": "bind",
        "source": "/var/log/foo",
        "options": [
                "nosuid",
                "nodev",
                "strictatime"
        ]
}

However I got the following error:

2018-02-09_22:04:23.06490 container_linux.go:265: starting container process caused "process_linux.go:348: container init caused \"rootfs_linux.go:57: mounting \\\"/var/log/foo\\\" to rootfs \\\"/path/to/rootfs\\\" at \\\"/path/to/rootfs/var/log/foo\\\" caused \\\"no such device\\\"\""

I wasn't able to find the issue until I 1) used strace on my mount --bind call to see that the MS_BIND flag was being passed to the mount() syscall, and 2) added print debugging to runc to see that the unix.MS_BIND flag was not being sent. This led me to adding the "bind" option and it started working!

I'm not sure if this would have been obvious to others, but I'm wondering if runc should either add the flag for the user if it's missing or report clearer error feedback when the bind type is used without the bind option? I'm also not sure if there's a case where that behavior would be desired?

[cyphar]

I think "type": "bind" is a mistake in the way that we handle stuff in runc. In reality, bind-mount types are ignored by the mount(2) syscall so we shouldn't be looking at that field at all.

But if we're going to have special-casing for bind (which we do in rootfs_linux.go unfortunately) then we should at least make sure that we special case things properly, and give warnings if users are making common mistakes (I've made this mistake a few times as well).