Re: [PATCH v2] libselinux: use /proc/thread-self when available

[Stephen Smalley <sds@tycho.nsa.gov>]

On 07/13/2015 03:20 PM, Eric Paris wrote:
> On Mon, 2015-07-13 at 09:56 -0400, Stephen Smalley wrote:
>> On 07/13/2015 09:34 AM, Eric Paris wrote:
>>> Looks good to me.
>>>
>>> That issue also brings up problems with glibc pid caching, right? 
>>> Do we
>>> need to rip out my *con cache entirely? I remember there was a
>>> measurable perf win with the cache, but it has proved a PITA 
>>> multiple
>>> times. Keeping the cache and fixing/working around the glibc pid
>>> caching would mean a tradeoff using getpid(2) instead of getpid(3),
>>> even with this patch. Right? Or maybe storing a ctid instead of 
>>> cpid
>>> and using gettid(2)...
>>>
>>> But this patch is LGTM.
>>
>> From the discussion in that issue, it sounded like switching to using
>> /proc/thread-self would avoid the problem without needing to switch 
>> to
>> getpid(2), but I have not tested systemd-nspawn --selinux-context 
>> after
>> the change to confirm this fact.  Certainly it ensures that we always
>> open the right path on Linux 3.17 and later.
>>
>> I would love to rip out the cache, but I didn't think we were free to 
>> do
>> so without first reworking coreutils in some manner to avoid the
>> repeated getfscreatecon()...setfscreatecon() calls made when copying
>> directory trees.
> 
> You might be right, but I'm not so sure. After a direct call to clone
> the cache has both a cpid and tid saved. So the (incorrect) cached tid
> you just fixed. Certainly possible this was the only problem systemd
> was having.
> 
> But the {get,set}procattrcon_raw() functions also have that cached
> cpid. Which is wrong after clone. Which means they will use the cached
> values instead of even getting to openattr()...  So maybe systemd
> hasn't caused anything to get cached and it will work by change, but
> the library is still busted against some users of clone...
> 
> I was going to suggest telling systemd to call free_procattr() after
> they call their own clone. But even that is busted since it uses
> getpid() and libc is going to cache that result.
> 
> Seems like we need to s/getpid(3)/getpid(2)/ in that whole file to make
> it right...

None of the /proc/self/attr attributes change on fork, only on exec.
So why do we need to flush the cache then?