This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] explicit_bzero final


On 12/15/2016 12:25 AM, Jeff Law wrote:
> On 12/14/2016 04:11 PM, Zack Weinberg wrote:
...
>>
>> If you compile this on x86-64 with gcc 6.2 at -DNO_CLEAR -O2 -S, you
>> will see that the `key` variable lives entirely in registers.  If you
>> compile it with neither `-DNO_CLEAR` nor `-DINLINE_EXPLICIT_BZERO`,
>> `key` is spilled to the stack immediately after the call to `get_key`
>> (it's more obvious that this is what's happening if you turn off RTL
>> scheduling). This is what we want to avoid.  With
>> `-DINLINE_EXPLICIT_BZERO` ... the spill still happens,
>> disappointingly, but it _could have_ been eliminated, because those
>> stack slots are not read again before they are overwritten with zeroes
>> by the memset.  (LLVM 3.8 does eliminate the spill.)
>
> Right.  Just to correct a bit on terminology, it's not spilled to the
> stack, but the object has to be addressable due to the call to
> explicit_bzero.  That forces the object into the stack.  It might still
> wind up in the stack due to other reasons (it's an aggregate and thus
> not subject to live in registers).

Right.  I suspect that sensitive data objects pretty much always wind up
in memory anyway, but people _were_ really worried about the case where
explicit_bzero is the only thing that makes one addressable.

...
> But the fact remains that as long as KEY is addressable you're going to
> have this class of problem and with the explicit_bzero proposal anything
> passed to it will be considered addressable and is likely going to drop
> into memory, which is precisely what you do not want.
> 
> Rather than explicit_bzero what you may want is something more like a
> clobbering assignment.

To be clear, explicit_bzero itself is not going away.  It is already in
use in a wide variety of applications, and substitutes (such as the
hypothetical __attribute__((sensitive))) will not become widespread
quickly enough to avoid adding it to glibc.  I've been working on this
patch off and on for _two years_, and I picked it up from someone else
who'd given up on the process after roughly that long himself.  I fully
intend to commit it in some form tomorrow evening; I consider missing
2.25 unacceptable.

What I'd hope we could do with compiler-side smarts is _convert_
explicit_bzero to __attribute__((sensitive)) or a clobbering assignment
or whatever the most convenient compiler-side representation winds up
being, so that artifacts of glibc's implementation become irrelevant.

>> With a compiler that implemented the taint analysis I am imagining, we
>> would have instead
>>
>> encrypt (char * out, const char * in, size_t n)
>> {
>>   struct k key __attribute__ ((tainted));
>>
>>   <bb 2>:
>>   key = get_key ();
>>   do_encrypt (key, out_3(D), in_4(D), n_5(D));
>>   memset (&key, 0, 16);
>>   key = {0, 0};
>>   USE (key);
>>   key ={v} {CLOBBER};
>>   return;
>> }
>
> So if you get to this point, in particular the key = {0, 0} assignment,
> then what's the point of the explicit_bzero/memset call?  It's dead.
> What you're describing with key = {0, 0} is an empty CONSTRUCTOR
> assignment.  And I *think* those won't force your object to be addressable.
> 
> FWIW, if you had this, there's a good chance the compiler would pick up
> the fact that the memset call is dead, but I really don't see the point
> in having the memset call at that point.

This is in the hypothetical where we keep the inline expansion in glibc,
gcc recognizes __explicit_bzero_fence, and gcc doesn't try to
pattern-match the preceding memset.  So the __explicit_bzero_fence got
replaced with the clobbering assignment, but the memset call was already
there.

I'm a lot less attached to the inline expansion now that I realize it
doesn't keep 'key' from getting written to the stack in GCC 6, and
Florian has figured out how to make __explicit_bzero_chk happen.

> So why not drop explicit_bzero and instead we define something to get
> you an empty CONSTRUCTOR assignment.  I think those have the semantics
> you want without forcing your object to be addressable.

Again, explicit_bzero itself (or another name for the same operation) is
the interface applications are using right now.  It's not going away.
But __builtin_explicit_bzero could turn into that empty CONSTRUCTOR
assignment, couldn't it?

> Then you just have to find a way to deal with the argument passing and
> register allocation artifacts, and that's going to be hard I suspect.

Yeah.  But hey, at least it won't involve reload, right?

zw


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]