Changing back to per-thread credentials on Linux (fixing native AIO).
realrichardsharpe at gmail.com
Wed Jun 27 11:00:17 MDT 2012
On Wed, Jun 27, 2012 at 9:51 AM, Jeremy Allison <jra at samba.org> wrote:
> Hi all,
> Back in 2008 tridge noticed a lost-wakeup problem with our
> implementation using POSIX aio on Linux.
> It was an interesting bug :-). The problem is that the
> calls we used back then to change credentials (setresuid()
> and setgroups()) are non-atomic on glibc. This is because
> the underlying system calls such as setresuid() on Linux
> only apply per-thread, not per process (as POSIX requires).
> glibc solves this by keeping an internal list of all threads
> created via the pthread calls, and attaching a signal
> handler to every thread which cannot be masked or
> replaced by code external to glibc (glibc masks off
> such requests). When any thread calls setresuid() then
> glibc walks the list of known threads, sets up a
> command block describing the uid change and then
> sends the internal signal to every thread. The
> handler for this signal then makes the raw system
> call on behalf of the thread to change the credentials
> for that thread only. You can see this in the glibc
> source code by doing a grep for SETXID - it's very
> clever code (as is much of glibc :-).
> Now to the lost wakeup problem.
> glibc aio spins off a pthread to handle the aio reads
> or writes, and then uses posix real-time signals to
> notify the parent thread when the IO is done.
> Unfortunately for Samba, smbd can be in the process
> of changing from to a non-root user using the setresuid()
> call when the IO completes. The race is this - the
> aio thread has received the signal to change its
> credentials, and does so, whilst the glibc call
> is still walking the list of threads and has
> not yet changed the credential on the calling
> thread. Thus we can have the case where the
> initiator of the aio is running as root, whilst
> the completing aio thread has been switched to
> another user. That thread then has no permissions
> to signal its initiating thread to tell it that
> the aio is complete - hence the lost wakeup.
> It's rare :-). But does happen.
> Back in 2008 tridge fixed this in a very clever
> way (of course :-) by changing our autoconf
> tests to prefer setreuid() over setresuid().
> The glibc code for setreuid() did not do the
> SETXID signaling, whereas the glibc code for
> setresuid() did - so he sidestepped the problem
> and smbd continued using thread-specific
> credentials under the covers.
> However, time moves on :-). It's 2012 now and
> glibc has fixed *all* the setXuid() calls to
> use SETXID signaling internally, and so we are
> no longer safe from the lost wakeup.
> So here is a patch that fixes the problem on
> Linux+glibc by adding libreplace functions
> only on Linux that wrap all the possible
> setXuid/setXgid/setroups calls and cause
> them to bypass glibc and call into the
> kernel directly. I've also added a test
> suite (based on the code in source/lib/util_sec.c)
> that (when run as root) ensures that all uid/gid
> credential handling is done correctly on Linux
> using the new wrapper functions.
> It applies on master, and integrates cleanly
> with the uid_wrapper code (doesn't need to
> change it actually :-) by which I think it's
> a clean way of doing this.
> Comments please ! I'm planning to push this
> soon if people are ok with is and I'll create
> a bug to get it back-ported for 3.6.x (and
> maybe 3.5.x) to fix the lost-wakeup issue
> with Linux glibc aio.
> It also makes it much easier to proceed
> with the threaded aio open code I was working on,
> as now I can remove all the ugly race-condition
> handling code I was figting with, as we
> know that any thread can set its own credentials
> independently, ensuring no security issues
> on file create - but that's patch for another
> day :-).
Does this also mean that the pre-forked smbds with each smbd handling
many connections will also work on Linux, since we are using Linux'
ability to do per-thread credentials in violation of POSIX?
More information about the samba-technical