nspawn --selinux-context call to setexeccon fails because tid is cached and not invalidated

[richardmaw-codethink]

I've been trying to use the advanced nspawn features and came across a snag.
setexeccon() is failing with ENOENT.
After digging in with strace, it appears to be attempting to write to the exec context of the wrong thread.

I believe the reason for this is that libselinux caches this path, and invalidates it with an atfork() handler.
However, because glibc doesn't expose a low-level clone() interface, systemd does its own raw clone, hence the atfork() handlers are not called, so setexeccon() attempts to change the context of a process that does not exist.

I think our options are to either forego use of libselinux and write the context directly (I've tried this locally and it appears to work) or rework the uses of raw_clone to use the thread-centric clone provided by glibc.

[poettering]

Urks, libselinux is a pile of borkedness anyway. atfork() handlers really should be forbidden... They pretty much use everything that is ugly, like gcc constructors and suchlike.

I would be willing to take a patch that writes the context directly.

An alternative would be to change libselinux to query the tid directly, with syscall(). Or alternatively fix the glibc to either provide a proper wrapper for clone(), to provide a way to flush the cache of the tid, or remove the caching altogether.

@rhatdan do you have an opinion on this? Would you prefer to see this fixed in libselinux? or are you ok, if we simply forego libselinux for this one?

[danielsilverstone-ct]

I worked with Richard on this problem and for our ¢2 -- changes to libselinux or glibc take much much longer to reach usability, particularly when "Enterprise" linux is involved. Given that systemd already uses access to the proc filesystem directly for reading attributes of other processes (something libselinux in theory provides) for determining credentials of bus endpoints, my vote (were I to be afforded one) would be to directly access the attribute files here.

[poettering]

Usually we try at least to fix things properly where the issues are. Which in this case are somewhere between glibc/libselinux I think. Again, I am open to merge a patch to nspawn, that works around this borkedness in glibc/libselinux, but I'd really prefer to have @rhatdan's opinion on this first, whether he things this should be fixed in libselinux instead, and is willing to ensure this is done.

[richardmaw-codethink]

I've included #483 for completeness. I'm happy to keep this downstream if working-around the problem is not acceptable upstream.

[rhatdan]

We are having the conversation over at #483 but I would prefer to have @eparis comment on this. The patch looks fine but it seems to be either a bug in glibc or libselinux.

[eparis]

There's no great answer.... Someone could put a patch into libselinux to use getpid(2) instead of getpid(3). Would still be a lot cheaper than the syscalls to actually open the selinuxfs files and read/write the contents over and over and over and over....

[eparis]

@pcmoore think you would be willing to do that? change procattr.c to use getpid(2) and avoid the glibc cache of pid altogether?

[pcmoore]

Without really looking closely at the libselinux code, it sounds reasonable to me. However, I'm going to defer this to @stephensmalley since I'm more than a bit swamped at the moment and Stephen generally handles the userspace side anyway.

[stephensmalley]

SELinux discussions are supposed to occur on selinux@tycho.nsa.gov. Not buried in some github issues tracker. Willing to test a patch that switches to getpid(2). However, I'd like to revisit the justification for the procattr cache, as it seems an unending series of these fixes has been needed ever since it was first added and it keeps getting more complicated. Maybe the better question is why is coreutils calling getfscreatecon()...setfscreatecon() over and over and over again, especially when it isn't truly changing anything. Also interested in switching over to /proc/thread-self on kernels that support it.

[poettering]

/proc/thread-self sounds like a good solution for me, because glibc's fucked caching getpid() won't be used at all then.

Instead of changing libselinux to bypass glibc's getpid() it would also work for me us to first use /proc/thread-self and on ENOENT fallback to the glibc getpid() path... This would fix things for us at least on new kernels > 3.16, even if it would stay broken on older kernels.

[richardmaw-codethink]

If we're going to open-code setexeccon() for the time being in systemd-nspawn, then unless we anticipate needing threads, it should be sufficient to use the /proc/self/ path rather than the /proc/thread-self/ path.