pipe: Fall back to write() on vmsplice() EPERM

[ymanton]

vmsplice can be blocked via seccomp and currently is in Podman containers. CRIU uses vmsplice a lot on the checkpoint side, so it's easier to just run with seccomp=unconfined in that case. On the restore side I've only seen one use in restore_pipe_data being reached, so rather than forcing users to run with a custom profile or disable seccomp altogether it might be better to just fall back to calling write, which this patch does.

[0x7f454c46]

Hm, this seems like a workaround for a specific environment setup, rather than for a kernel code. Unsure if this should be hardcoded like this. Maybe an option in criu.config? This will be a policy/config-based decision and in result CRIU won't try hard on vmsplice() potentially causing notifications and issuing syscalls that are expected to fail, decreasing the performance as well.

[avagin]

@ymanton I look at the list [1] and can't firgure out why they decide to block vmsplice. It looks like a mistake. [1] https://github.com/containers/podman/blob/main/vendor/github.com/containers/common/pkg/seccomp/seccomp.json#L84

[avagin]

https://github.com/containers/common/commit/7ced5daafa0e36102eb931050ba3ff99f42bdfac

[ymanton]

containers/common@7ced5da

Thanks.

The discussion on lkml is very long and deep in kernel internals that I don't understand, but from what I can gather there is some worry that vmsplice can be abused, and I guess the Podman folks have blocked it to be on the safe side.

https://lore.kernel.org/linux-mm/X+Kxy3oBMSLz8Eaq@redhat.com/:

The problem is we have an unprivileged long term GUP like vmsplice that facilitates elevating the page count indefinitely, until the parent finally writes a secret to it. Theoretically a short term pin would do it too so it's not just vmpslice, but the short term pin would be incredibly more challenging to become a concern since it'd kill a phone battery and flash before it can read any data.

So what happens with your page_mapcount == 1 check is that it doesn't mean non-COW (we thought it did until it didn't for the long term gup pin in vmsplice).

Jann's testcases does fork() and set page_mapcount 2 and page_count to 2, vmsplice, take unprivileged infinitely long GUP pin to set page_count to 3, queue the page in the pipe with page_count elevated, munmap to drop page_count to 2 and page_mapcount to 1.

page_mapcount is 1, so you'd think the page is non-COW and owned by the parent, but the child can still read it so it's very much still wp_page_copy material if the parent tries to modify it. Otherwise the child can read the content.

https://lore.kernel.org/linux-mm/X+PoXCizo392PBX7@redhat.com/:

I already told Jens, is io_uring could use mmu notifier already (that would make any io_uring GUP pin not count anymore with regard to page_mapcount vs page_count difference) and vmsplice also needs some handling or maybe become privileged.

So unless you guys think otherwise I doubt we will convince the Podman folks to unblock it any time soon, and some others might follow in their footsteps.

I don't mind making it a config option. Do we want an option that avoids all uses of vmsplice on both checkpoint and restore? Or should we consider just getting rid of it altogether?

[Snorch]

Can we in CRIU temporary disable this seccomp rule while restoring (e.g. only for restored container) or even restore seccomp rules later after vmsplicing everything we need to?

[avagin]

@Snorch I think it is about unprivileged C/R. In this case, we can't suspend seccomp.

[github-actions[bot]]

A friendly reminder that this PR had no activity for 30 days.