re: disabling FORTIFY_SOURCE

[Dan Kegel]

Hi Kees,
sure, supporting Fortify in Wine would be great, but it's not
clear how long it will take to fix Wine so it works with Fortify.

Which would you prefer:
1) have Wine broken for an unknown and possibly long time
or
2) have Wine working, but without Fortify, until the bugs are fixed
?

disabling FORTIFY_SOURCE

[Marcus Meissner]

On Sat, Oct 23, 2010 at 11:54:27PM +0000, Dan Kegel wrote:
> Hi Kees,
> sure, supporting Fortify in Wine would be great, but it's not
> clear how long it will take to fix Wine so it works with Fortify.
>
> Which would you prefer:
> - have Wine broken for an unknown and possibly long time
>
> or
> 2) have Wine working, but without Fortify, until the bugs are fixed
> ?

Actually I would like to know if its just more than the dlls/shell32/pidl.c
problem...

(And of course also the stupid warn_unused_results warning spam, which we
disabled already.)

Ciao, Marcus

disabling FORTIFY_SOURCE

[Dmitry Timoshkov]

Marcus Meissner  wrote:
> Actually I would like to know if its just more than the dlls/shell32/pidl.c
> problem...

It's the problem with any storage declared as something[1], there are plenty
of them in win32, and that's perfectly valid code.
> (And of course also the stupid warn_unused_results warning spam, which we
> disabled already.)

It's also infamous "longjmp causes uninitialized stack frame":
http://bugs.winehq.org/show_bug.cgi?id=21405

disabling FORTIFY_SOURCE

[Marcus Meissner]

On Sun, Oct 24, 2010 at 03:44:35PM +0900, Dmitry Timoshkov wrote:
> Marcus Meissner  wrote:
> > Actually I would like to know if its just more than the dlls/shell32/pidl.c
> > problem...
>
> It's the problem with any storage declared as something[1], there are plenty
> of them in win32, and that's perfectly valid code.

Actually FORTIFY knows those constructs in standalone structs and handles
them correctly.

It is only if they are embedded within another struct or unions when it
has problems.

Ciao, Marcus

disabling FORTIFY_SOURCE

[Vitaliy Margolen]

On 10/24/2010 12:32 AM, Marcus Meissner wrote:
> Actually I would like to know if its just more than the dlls/shell32/pidl.c
> problem...

If you take a look at winternl.h you'll see number of structures there look 
like:
typedef struct _foo {
     ULONG length;
     WCHAR buffer[1];
} foo, *pfoo;

Or just grep for '[1]' in include directory. Lots and lots of declarations 
in all different places.

Vitaliy.

disabling FORTIFY_SOURCE

[Marcus Meissner]

On Sun, Oct 24, 2010 at 09:50:42AM -0600, Vitaliy Margolen wrote:
> On 10/24/2010 12:32 AM, Marcus Meissner wrote:
> > Actually I would like to know if its just more than the dlls/shell32/pidl.c
> > problem...
>
> If you take a look at winternl.h you'll see number of structures
> there look like:
> typedef struct _foo {
>     ULONG length;
>     WCHAR buffer[1];
> } foo, *pfoo;
>
> Or just grep for '[1]' in include directory. Lots and lots of
> declarations in all different places.

As I already wrote, this works.

Here is a sample code which shows the problem dlls/shell32/pidl.c has:

$ cat xx1.c
    #include <string.h>
    #include <stdlib.h>

struct foo {
    	int x;
    	char y[1];
    };
    union bar {
    	struct foo fo;
    	long y;
    	float fl;
    };

struct berk {
    	int t;
    	union bar b;
    };

int
    main(int argc, char **argv) {
    	struct berk *x1;
    	struct foo  *x2;

x1 = malloc (sizeof(struct berk) + 5);
    	x2 = malloc (sizeof(struct foo)  + 5);
    	strcpy(x1->b.fo.y, "hallo");
    	strcpy(x2->y, "hallo");
    }

$ gcc -O2 -Wall -D_FORTIFY_SOURCE=2 -g xx1.c -o xx1
    xx1.c: In function ‘main’:
    xx1.c:28:1: warning: control reaches end of non-void function
    In file included from /usr/include/string.h:640:0,
    		 from xx1.c:1:
    In function ‘strcpy’,
        inlined from ‘main’ at xx1.c:26:8:
    /usr/include/bits/string3.h:107:3: warning: call to __builtin___strcpy_chk will always overflow destination buffer

Only the "strcpy(x1->b.fo.y, "hallo");" with the nested struct is warned
about, while the second strcpy works fine.

It is just nested structs it does not like at this time.

Ciao, Marcus

disabling FORTIFY_SOURCE

[James McKenzie]

On 10/24/10 8:50 AM, Vitaliy Margolen wrote:
> On 10/24/2010 12:32 AM, Marcus Meissner wrote:
> > Actually I would like to know if its just more than the 
> > dlls/shell32/pidl.c
> > problem...
>
> Or just grep for '[1]' in include directory. Lots and lots of 
> declarations in all different places.

Stoopid question time.  Is there a better method of declaring these than 
making them 2 char arrays?  Can we 'assume' a maximum length and set 
them to that to prevent possible buffer overflows or is this something 
that we have to accept and work with?  I understand that we want to keep 
the memory footprint for Wine as low as possible but we also don't want 
to introduce possible exploits in the process.

James McKenzie