[erlang-bugs] R14B01: buffer overflow detected during compilation with -D_FORTIFY_SOURCE=2 (x86_64)

[pan@REDACTED]

Hi,

I've looked into it and found what _FORTIFY_SOURCE does not like.

The construct name[1] is as Mikael explained to later dynamically allocate 
larger arrays, a common "trick" used throughout the VM to allow structures 
with different sized buffers. C99 allows structures to contain arrays with 
*no* size last in a struct, but the VM is not written in C99, as Mikael 
also correctly pointed out. We are however discussing using this C99 
feature in future releases as very few people use a gcc or a MSVC++ that 
is too old to have that feature. GCC i think has all of C99 implemented 
and so does MSVC++, at least in the latest release. One of the reasons to 
use [] instead of [1] is the over-allocation Mikael pointed out, which 
happens regardless of unions as the sizeof() struct returns an aligned 
size.

Anyway, the problem here is that the name field is actually not the last 
field in the struct, which is plain wrong. It does not actually produce 
any real buffer overflow, but the member b of the surrounding struct is 
the one which should be used for the dynamic buffer. This is old code that 
is really strange, but has worked despite that (b is never accessed when 
name is used, so it only generates even more over-allocation). It now got 
exposed as I started to use strcpy on the field when i implemented unicode 
filenames. Didn't notice the strange name field when i rewrote it though.

So - it is in reality no buffer overflow. In fact, given that the last 
field is used, as the code intended, -D_FORTIFY_SOURCE allows this kind of 
constructs. It does not complain if the last member of a structure is 
indexed outside it's bounds and the actually allocated area is large 
enough.

So, any workaround to silence -D_FORTIFY_SOURCE will do the trick, there 
is no real buffer overflow. I've made the change to use the "b" field for 
this callback and removed the strange "name" field in the union, which 
also makes _FORTIFY_SOURCE happy and makes the code in some way more 
understandable. Changing to the C99 way will be made in the future, when 
I'm sure it wont cause problems for people with strange/old platforms out 
there. That change will however touch a lot of structures in the VM as 
this kind of code is used at several places (for performance reasons).

The change that fixes this will be visible at github as soon as out daily 
builds has accepted it, most probably before the end of the week.

Cheers,
/Patrik