10 ms·

At Microsoft the compiler team (Visual C++) and the Windows team are joined at the hip. I'm sure the same was true at Sun. This can lead to good decisions about

At Microsoft the compiler team (Visual C++) and the Windows team are joined at the hip. I'm sure the same was true at Sun. This can lead to good decisions about undefined behavior that I hope would make Linus smile. I recently learned of one such good engineering decision (I hope I'm remembering it correctly). Let's say you have a struct with an int32 and a byte in it. That's 5 bytes, right? But the platform alignment is a multiple of 4 bytes, so there's 3 bytes of padding (sizeof the struct is 8 bytes). If we stack-allocate an array of 11 of these and zero-initialize with = { 0 }, what would you expect to see in memory after initialization? It turns out the answer was that the first element of the array would have its 5 bytes zeroed, but the 3 bytes of padding would be left uninitialized. Then, the remaining 10 elements of the array would be zeroed with a memset that actually zeroed all 80 remaining bytes. It sounds weird but this is a legal thing to do from the standard's perspective. All they're obligated to zero out are the non-padding bytes. This UB was leading to disclosure of little bits of kernel memory back into user mode because Windows engineers assumed that = { 0 } was the same as leaving the variable uninitialized and then memsetting the whole thing to zero. Nope! The compiler team fixed this by always zeroing out padding too. Problem solved. There are some cases where it's not quite as fast. But it's the right engineering decision by the compiler team for their customers, both internal and external.

oblong 8y ago
> This UB was leading to disclosure of little bits of kernel memory back into user mode because Windows engineers assumed that = { 0 } was the same as leaving the variable uninitialized and then memsetting the whole thing to zero But what on earth were they doing with the padding bits?
- kabdib 8y ago
  Total guess, but they could have been including them in a hash function or struct equality check (equality with memcmp, hash by just grabbing bytes, etc.). That would not have gone well with uninitialized padding :-)
- buckminster 8y ago
  It's not uncommon for a function to use a supplied output buffer as scratch space. So the padding could have contained pretty much anything.
- Gibbon1 8y ago
  They aren't doing anything with the padding bytes. But what probably happened was they copied the array into userland memory. Which potentially allows a malicious sandboxed program to read the padding bytes that contain bits of kernal memory.
pjmlp 8y ago
Now that Microsoft is having again a new found love in C, either via Checked C research, the UDMF rewrite in C, WSL, or the upcoming Azure Sphere product, it would be interesting if they could contribute to improving the whole security story in C2X.
- slededit 8y ago
  I don't think they are really that much in love with C aside from a few niche areas. The compiler itself still generates worse code when in C mode as it only exists for back-compat. When I worked there it was explicitly said that the new "universal" (or whatever they are called now) APIs were explicitly not targeting C. C++ on the other hand seemed to be undergoing a new renaissance. They have added some newer C features but only those required by the C++ standard.
  - pjmlp 8y ago
    Quite true, when speaking about Visual C++ and I have also stated the same thing multiple times here. Their C love on the projects I mentioned, is via clang and gcc.
tlb 8y ago
It may be the right thing for the moment, but what happens to that code when new versions of the compiler come out? Someday it'll start leaking information again. Relying on non-standard behavior always ends in tears.
- 8_hours_ago 8y ago
  Presumably the compiler team wrote a test for that behavior so there won’t be a regression. I assume that Windows isn’t planning on switching compilers, so they can rely on that behavior as long as the Microsoft compiler passes all its tests.
- EpicEng 8y ago
  >Relying on non-standard behavior always ends in tears. Who's relying on unspecified behavior? Do you mean some theoretical programmer? Well of course that's not a good thing, but I don't see what it has to do with the situation in the story and the MS engineers decideing to change how they implement that unspecified part of the spec.
  - tlb 8y ago
    The kernel programmer did, in this story. An example of such an information leak is: struct foo { int a; char b; } void send_foo_01() { foo x {0, 1}; write(fd, &x, sizeof(foo)); } which sends 3 bytes of the contents of the stack memory over the network, for almost every compiler except the one in the story. Running in a kernel, that could contain secrets.
    - EpicEng 8y ago
      Sorry, my mistake; yes, the kernel dev is now relying on the compiler to zero out those three bytes. I understand the decision to change this in the compiler, but I think the "fix" would have been to memset in the kernel code. I'd be surprised if they didn't do that, but maybe they can reasonably assume they'll never use another compiler to build the Windows kernel.
  - legulere 8y ago
bigcheesegs 8y ago
The problem with this kind of approach is over time it removes the ability to use any other implementation. You are no longer using C, you are using <Implementation>C. This becomes a problem when a different implementation adds amazing tools for finding bugs (the sanitizer suite, for instance), and you can't use them because your code doesn't build with any other implementation.
- iforgotpassword 8y ago
  Luckily clang does a good job at being GCC compatible, so I don't worry too much about using GCC extensions. It's quite unlikely one of them will go away anytime soon, and theory pretty much cover all architectures/platforms that have ever existed.
  - pjmlp 8y ago
    Usually true, unless you want to target embedded, mainframes or some industrial OSes.
  - saagarjha 8y ago
    It doesn't support everything; for example, Clang doesn't do nested functions.
- pjmlp 8y ago
  This was pretty much how writting C code between K&R, ANSI C89 and subsets e.g. Small-C used to look like.
- 8_hours_ago 8y ago
  Large and complex pieces of software, and operating systems in particular, tend to be tightly tied to their compilers. It is never easy and in some cases practically impossible to port to a different compiler. I expect that Microsoft has come to terms with the fact that Windows will only support being compiled by their compiler. When a different toolchain introduces a new feature for finding bugs that would be useful for Windows, the Microsoft compiler team can add that functionality to their own tools instead of porting Windows. An advantage of this is that they can customize the feature for exactly their use case. Yes, this is the definition of NIH syndrome, but that’s how large companies work.
hermitdev 8y ago
Minor nitpick, the zeroing (or lack thereof) of the padding is not undefined behavior, it's unspecified behavior. Undefined behavior and unspecified behavior often look and perhaps behave the same to the programmer, but have semantic differences. In the face of undefined behavior, the compiler is allowed to do pretty much anything it wants (including formatting your hard drive and/or launching the nukes). With unspecified behavior, the compiler implementer must make a conscious decision on what the behavior will be and document the behavior it will follow.
- ythn 8y ago
  > and document the behavior it will follow. So it's unspecified in terms of the standard, but specified by the implementation
  - jexah 8y ago
    Right! Basically it's up to the compiler programmer to pick a path and follow it... assuming you're talking about implementation-defined.
- jcranmer 8y ago
  > With unspecified behavior, the compiler implementer must make a conscious decision on what the behavior will be and document the behavior it will follow. No, what you described is implementation-defined behavior. It may be confusing, but here's the breakdown of different kinds of behavior in the C standard: * Well-defined: there is a set of semantics that is defined by the C abstract machine that every implementation must (appear to) execute exactly. Example: the result of a[b]. * Implementation-defined: the compiler has a choice of what it may implement for semantics, and it must document the choice it makes. Example: the size (in bits and chars) of 'int', the signedness of 'char'. * Unspecified: the compiler has a choice of what it may implement for semantics, but the compiler is not required to document the choice, nor is it required to make the same choice in all circumstances. Example: the order of evaluation of a + b. * Undefined: the compiler is not required to maintain any observable semantics of a program that executes undefined behavior (key point: undefined behavior is a dynamic property related to an execution trace, not a static property of the source code). Example: dereferencing a null pointer.
deleted 8y ago
[deleted]
minipci1321 8y ago
> Problem solved. This "solution" also prevents code analysis tools from detecting reads from padding bytes as "non-initialized memory reads". When a routine leaves sensitive data on the stack, the entire area used up by that sensitive data, must be wiped out before the routine returns (and still, wiping out padding bytes would not be required). How about the part of the stack space not covered by that array of 11 elements?
- BeeOnRope 8y ago
  The kernel has a separate stack, inaccessible to user-space. Otherwise, you'd be right: a shared stack would be a giant source of information leakage from kernel space to user-space unless it was very carefully managed (probably at a significant performance cost). Thus, separate stacks (it also has the advantage of not needing to make assumptions about how user-mode programs use their stack, e.g., if they are transiently using "unallocated" stack above rsp, etc). Probably what happened here is that this structure was copied back to user space (e.g., as the result of a system call) exposing the kernel data.
skookumchuck 8y ago
> This UB was leading to disclosure of little bits of kernel memory back into user mode If you write inline assembler, you can access this stuff anyway. So I'm not seeing what the value is in zeroing it by the caller. The kernel callee should zero its stack frame before returning.
- BeeOnRope 8y ago
  How so? The kernel presumably has a separate stack which is not accessible to user-space, but here information was disclosed because a structure copied back to user space was create on the stack, initialized to {0} and then member-wise assigned, with some padding bytes never being touched and thus containing whatever previous values happened to be on the kernel stack. So far this is all in kernel-space so nothing been exposed yet. Then, however, if this structure is copied back to user-space, e.g,. as an output parameter of a kernel call, the padding bytes with the exposed data will be copied along with it (unless you get lucky and the copy routine happens to make the exact same decision with regard to padding handling). If the kernel stack _itself_ was visible to user-space, you'd have a whole separate set of problems: you'd have to zero the whole stack (or at least the extent of the stack that could have been touched) on every kernel call.
  - skookumchuck 8y ago
    Yes, you're right.
cryptonector 8y ago
At Sun Solaris and Studio were quite separated organizationally (and business-wise too). Mind you, the Solaris engineering group had early access to Studio and also, of course, access to the Studio team, so... not quite joined at the hip, but not too separated either.
robotresearcher 8y ago
It’s great that the compiler team could implement safer behaviour. But if the programmers’ intent was to zero all bits in the array they should express this explicitly in the code with a memset(). Otherwise a change in the compiler later could throw up this vulnerability again. The code should express the semantics as clearly as possible.
- adrianratnapala 8y ago
  Then other other language lawyers will come around and tell you why you should use {0} instead of memset (e.g. because for some combinations of type and architecture the zero value isn't full of zero bytes). This example also shows how "the semantics" is a fiddly concept. The reason the standard allows leaving bytes unzeroed is because they are not "semantically" important. But they actually do matter. The problem with the mentality that it's always the programmer fault for not following "the rules" is they you eventually get to the point where the rules allow for no good solutions at all.
  - robotresearcher 8y ago
    I didn't say it was always the programmers' fault. But I believe the programmer should express their intent as clearly as possible. And memset( (void*)buf, 0, buflen ) says fill-a-contiguous-array-of-unsigned-chars-of-value-zero, which is semantically different from initializing an array of structs that may have padding, and better matches the programmers' intent. It doesn't matter if the zero value is all 0 bits or not - the important thing is that the whole contiguous memory region is zeroed. I believe C99 says chars and unsigned chars have no padding. https://stackoverflow.com/questions/13929462/can-the-unsigned-char-type-have-padding-bits-and-or-unused-values https://stackoverflow.com/questions/13929462/can-the-unsigne...
  - robotresearcher 8y ago
    Another, better, response to your argument. Initializing an array of structs with = {0} does NOT tell the compiler that zeroing the entire contiguous chunk of memory matters - only that each struct should be zeroed. While memset( (void*)buf, 0, len) does tell the compiler that the entire contiguous memory chunk must be zeroed, which is what is intended. Language lawyers need not apply.
xvilka 8y ago
Well, MSVC team is the last one who can propose something since they didn't even support C11 completely today, and weren't able to add C99 support for almost two decades.
- nwellnhof 8y ago
  They still don't support C99 fully.
  - pjmlp 8y ago
    They do, to the extent required by ANSI C++14, ANSI C++17 upgraded the requirements to C11. They are pretty open the future of systems programming on Windows is C++ and .NET Native. For C devs, they have helped clang to work on Windows and there is WSL for UNIX like software. Just like they have improved Visual Studio integration with clang and gcc. As for Visual C++, the name says it all.
captain_perl 8y ago
I'm skeptical that it was implemented that way because of kernel memory leaking concerns. More likely it was done to prevent Windows API calls from panicking when they accessed unset parameter structures. As a former Windows programmer, that was one of the largest sources of errors back in the Win32 days.
- jclulow 8y ago
  But you don't access the padding; it's generally implicitly added by the compiler to comply with the ABI rules for the platform. That argument might make sense if this was about zeroing _all_ stack objects not explicitly initialised, but it's explicitly talking about the padding.
nwellnhof 8y ago
In C11, the spec was changed to make zeroing out padding mandatory.

oblong 8y ago

> This UB was leading to disclosure of little bits of kernel memory back into user mode because Windows engineers assumed that = { 0 } was the same as leaving the variable uninitialized and then memsetting the whole thing to zero But what on earth were they doing with the padding bits?

kabdib 8y ago
Total guess, but they could have been including them in a hash function or struct equality check (equality with memcmp, hash by just grabbing bytes, etc.). That would not have gone well with uninitialized padding :-)
buckminster 8y ago
It's not uncommon for a function to use a supplied output buffer as scratch space. So the padding could have contained pretty much anything.
Gibbon1 8y ago
They aren't doing anything with the padding bytes. But what probably happened was they copied the array into userland memory. Which potentially allows a malicious sandboxed program to read the padding bytes that contain bits of kernal memory.

pjmlp 8y ago

Now that Microsoft is having again a new found love in C, either via Checked C research, the UDMF rewrite in C, WSL, or the upcoming Azure Sphere product, it would be interesting if they could contribute to improving the whole security story in C2X.

slededit 8y ago
I don't think they are really that much in love with C aside from a few niche areas. The compiler itself still generates worse code when in C mode as it only exists for back-compat. When I worked there it was explicitly said that the new "universal" (or whatever they are called now) APIs were explicitly not targeting C. C++ on the other hand seemed to be undergoing a new renaissance. They have added some newer C features but only those required by the C++ standard.
- pjmlp 8y ago
  Quite true, when speaking about Visual C++ and I have also stated the same thing multiple times here. Their C love on the projects I mentioned, is via clang and gcc.

tlb 8y ago

It may be the right thing for the moment, but what happens to that code when new versions of the compiler come out? Someday it'll start leaking information again. Relying on non-standard behavior always ends in tears.

8_hours_ago 8y ago
Presumably the compiler team wrote a test for that behavior so there won’t be a regression. I assume that Windows isn’t planning on switching compilers, so they can rely on that behavior as long as the Microsoft compiler passes all its tests.
EpicEng 8y ago
>Relying on non-standard behavior always ends in tears. Who's relying on unspecified behavior? Do you mean some theoretical programmer? Well of course that's not a good thing, but I don't see what it has to do with the situation in the story and the MS engineers decideing to change how they implement that unspecified part of the spec.
- tlb 8y ago
  The kernel programmer did, in this story. An example of such an information leak is: struct foo { int a; char b; } void send_foo_01() { foo x {0, 1}; write(fd, &x, sizeof(foo)); } which sends 3 bytes of the contents of the stack memory over the network, for almost every compiler except the one in the story. Running in a kernel, that could contain secrets.
  - EpicEng 8y ago
    Sorry, my mistake; yes, the kernel dev is now relying on the compiler to zero out those three bytes. I understand the decision to change this in the compiler, but I think the "fix" would have been to memset in the kernel code. I'd be surprised if they didn't do that, but maybe they can reasonably assume they'll never use another compiler to build the Windows kernel.
- legulere 8y ago

bigcheesegs 8y ago

The problem with this kind of approach is over time it removes the ability to use any other implementation. You are no longer using C, you are using <Implementation>C. This becomes a problem when a different implementation adds amazing tools for finding bugs (the sanitizer suite, for instance), and you can't use them because your code doesn't build with any other implementation.

iforgotpassword 8y ago
Luckily clang does a good job at being GCC compatible, so I don't worry too much about using GCC extensions. It's quite unlikely one of them will go away anytime soon, and theory pretty much cover all architectures/platforms that have ever existed.
- pjmlp 8y ago
  Usually true, unless you want to target embedded, mainframes or some industrial OSes.
- saagarjha 8y ago
  It doesn't support everything; for example, Clang doesn't do nested functions.
pjmlp 8y ago
This was pretty much how writting C code between K&R, ANSI C89 and subsets e.g. Small-C used to look like.
8_hours_ago 8y ago
Large and complex pieces of software, and operating systems in particular, tend to be tightly tied to their compilers. It is never easy and in some cases practically impossible to port to a different compiler. I expect that Microsoft has come to terms with the fact that Windows will only support being compiled by their compiler. When a different toolchain introduces a new feature for finding bugs that would be useful for Windows, the Microsoft compiler team can add that functionality to their own tools instead of porting Windows. An advantage of this is that they can customize the feature for exactly their use case. Yes, this is the definition of NIH syndrome, but that’s how large companies work.

hermitdev 8y ago

Minor nitpick, the zeroing (or lack thereof) of the padding is not undefined behavior, it's unspecified behavior. Undefined behavior and unspecified behavior often look and perhaps behave the same to the programmer, but have semantic differences. In the face of undefined behavior, the compiler is allowed to do pretty much anything it wants (including formatting your hard drive and/or launching the nukes). With unspecified behavior, the compiler implementer must make a conscious decision on what the behavior will be and document the behavior it will follow.

ythn 8y ago
> and document the behavior it will follow. So it's unspecified in terms of the standard, but specified by the implementation
- jexah 8y ago
  Right! Basically it's up to the compiler programmer to pick a path and follow it... assuming you're talking about implementation-defined.
jcranmer 8y ago
> With unspecified behavior, the compiler implementer must make a conscious decision on what the behavior will be and document the behavior it will follow. No, what you described is implementation-defined behavior. It may be confusing, but here's the breakdown of different kinds of behavior in the C standard: * Well-defined: there is a set of semantics that is defined by the C abstract machine that every implementation must (appear to) execute exactly. Example: the result of a[b]. * Implementation-defined: the compiler has a choice of what it may implement for semantics, and it must document the choice it makes. Example: the size (in bits and chars) of 'int', the signedness of 'char'. * Unspecified: the compiler has a choice of what it may implement for semantics, but the compiler is not required to document the choice, nor is it required to make the same choice in all circumstances. Example: the order of evaluation of a + b. * Undefined: the compiler is not required to maintain any observable semantics of a program that executes undefined behavior (key point: undefined behavior is a dynamic property related to an execution trace, not a static property of the source code). Example: dereferencing a null pointer.

deleted 8y ago

[deleted]

minipci1321 8y ago

> Problem solved. This "solution" also prevents code analysis tools from detecting reads from padding bytes as "non-initialized memory reads". When a routine leaves sensitive data on the stack, the entire area used up by that sensitive data, must be wiped out before the routine returns (and still, wiping out padding bytes would not be required). How about the part of the stack space not covered by that array of 11 elements?

BeeOnRope 8y ago
The kernel has a separate stack, inaccessible to user-space. Otherwise, you'd be right: a shared stack would be a giant source of information leakage from kernel space to user-space unless it was very carefully managed (probably at a significant performance cost). Thus, separate stacks (it also has the advantage of not needing to make assumptions about how user-mode programs use their stack, e.g., if they are transiently using "unallocated" stack above rsp, etc). Probably what happened here is that this structure was copied back to user space (e.g., as the result of a system call) exposing the kernel data.

skookumchuck 8y ago

> This UB was leading to disclosure of little bits of kernel memory back into user mode If you write inline assembler, you can access this stuff anyway. So I'm not seeing what the value is in zeroing it by the caller. The kernel callee should zero its stack frame before returning.

BeeOnRope 8y ago
How so? The kernel presumably has a separate stack which is not accessible to user-space, but here information was disclosed because a structure copied back to user space was create on the stack, initialized to {0} and then member-wise assigned, with some padding bytes never being touched and thus containing whatever previous values happened to be on the kernel stack. So far this is all in kernel-space so nothing been exposed yet. Then, however, if this structure is copied back to user-space, e.g,. as an output parameter of a kernel call, the padding bytes with the exposed data will be copied along with it (unless you get lucky and the copy routine happens to make the exact same decision with regard to padding handling). If the kernel stack _itself_ was visible to user-space, you'd have a whole separate set of problems: you'd have to zero the whole stack (or at least the extent of the stack that could have been touched) on every kernel call.
- skookumchuck 8y ago
  Yes, you're right.

cryptonector 8y ago

At Sun Solaris and Studio were quite separated organizationally (and business-wise too). Mind you, the Solaris engineering group had early access to Studio and also, of course, access to the Studio team, so... not quite joined at the hip, but not too separated either.

robotresearcher 8y ago

It’s great that the compiler team could implement safer behaviour. But if the programmers’ intent was to zero all bits in the array they should express this explicitly in the code with a memset(). Otherwise a change in the compiler later could throw up this vulnerability again. The code should express the semantics as clearly as possible.

adrianratnapala 8y ago
Then other other language lawyers will come around and tell you why you should use {0} instead of memset (e.g. because for some combinations of type and architecture the zero value isn't full of zero bytes). This example also shows how "the semantics" is a fiddly concept. The reason the standard allows leaving bytes unzeroed is because they are not "semantically" important. But they actually do matter. The problem with the mentality that it's always the programmer fault for not following "the rules" is they you eventually get to the point where the rules allow for no good solutions at all.
- robotresearcher 8y ago
  I didn't say it was always the programmers' fault. But I believe the programmer should express their intent as clearly as possible. And memset( (void*)buf, 0, buflen ) says fill-a-contiguous-array-of-unsigned-chars-of-value-zero, which is semantically different from initializing an array of structs that may have padding, and better matches the programmers' intent. It doesn't matter if the zero value is all 0 bits or not - the important thing is that the whole contiguous memory region is zeroed. I believe C99 says chars and unsigned chars have no padding. https://stackoverflow.com/questions/13929462/can-the-unsigned-char-type-have-padding-bits-and-or-unused-values https://stackoverflow.com/questions/13929462/can-the-unsigne...
- robotresearcher 8y ago
  Another, better, response to your argument. Initializing an array of structs with = {0} does NOT tell the compiler that zeroing the entire contiguous chunk of memory matters - only that each struct should be zeroed. While memset( (void*)buf, 0, len) does tell the compiler that the entire contiguous memory chunk must be zeroed, which is what is intended. Language lawyers need not apply.

xvilka 8y ago

Well, MSVC team is the last one who can propose something since they didn't even support C11 completely today, and weren't able to add C99 support for almost two decades.

nwellnhof 8y ago
They still don't support C99 fully.
- pjmlp 8y ago
  They do, to the extent required by ANSI C++14, ANSI C++17 upgraded the requirements to C11. They are pretty open the future of systems programming on Windows is C++ and .NET Native. For C devs, they have helped clang to work on Windows and there is WSL for UNIX like software. Just like they have improved Visual Studio integration with clang and gcc. As for Visual C++, the name says it all.

captain_perl 8y ago

I'm skeptical that it was implemented that way because of kernel memory leaking concerns. More likely it was done to prevent Windows API calls from panicking when they accessed unset parameter structures. As a former Windows programmer, that was one of the largest sources of errors back in the Win32 days.

jclulow 8y ago
But you don't access the padding; it's generally implicitly added by the compiler to comply with the ABI rules for the platform. That argument might make sense if this was about zeroing _all_ stack objects not explicitly initialised, but it's explicitly talking about the padding.

nwellnhof 8y ago

In C11, the spec was changed to make zeroing out padding mandatory.