* [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
@ 2008-09-24 18:53 Stefan Schulze Frielinghaus
2008-09-24 19:20 ` Andrew Thomas Pinski
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-09-24 18:53 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 302 bytes --]
The Stack Smashing Protector does not include functions with wide
character arrays. The following code wouldn't be protected:
#include <wchar.h>
int main (void) {
wchar_t buf[8];
wcscpy (buf, L"Hello World!");
return 0;
}
Attached patch solves this problem. Tested on i686
[-- Attachment #2: cfgexpand.c.patch --]
[-- Type: text/x-patch, Size: 763 bytes --]
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c (revision 140598)
+++ gcc/cfgexpand.c (working copy)
@@ -41,6 +41,7 @@
#include "tree-inline.h"
#include "value-prof.h"
#include "target.h"
+#include "c-common.h"
/* Return an expression tree corresponding to the RHS of GIMPLE
@@ -1201,7 +1202,10 @@
t = TYPE_MAIN_VARIANT (TREE_TYPE (type));
if (t == char_type_node
|| t == signed_char_type_node
- || t == unsigned_char_type_node)
+ || t == unsigned_char_type_node
+ || t == wchar_type_node
+ || t == signed_wchar_type_node
+ || t == unsigned_wchar_type_node)
{
unsigned HOST_WIDE_INT max = PARAM_VALUE (PARAM_SSP_BUFFER_SIZE);
unsigned HOST_WIDE_INT len;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
2008-09-24 18:53 [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays Stefan Schulze Frielinghaus
@ 2008-09-24 19:20 ` Andrew Thomas Pinski
2008-09-25 15:40 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Thomas Pinski @ 2008-09-24 19:20 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
You cannot include c-common.h in the cfgexpand.c as some languages
don't implement any thing in c-common.h.
Sent from my iPhone
On Sep 24, 2008, at 11:11 AM, Stefan Schulze Frielinghaus <stefan@seekline.net
> wrote:
> The Stack Smashing Protector does not include functions with wide
> character arrays. The following code wouldn't be protected:
>
> #include <wchar.h>
>
> int main (void) {
> wchar_t buf[8];
>
> wcscpy (buf, L"Hello World!");
>
> return 0;
> }
>
> Attached patch solves this problem. Tested on i686
> <cfgexpand.c.patch>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
2008-09-24 19:20 ` Andrew Thomas Pinski
@ 2008-09-25 15:40 ` Stefan Schulze Frielinghaus
2008-09-25 22:51 ` Andrew Pinski
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-09-25 15:40 UTC (permalink / raw)
To: Andrew Thomas Pinski; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On Wed, 2008-09-24 at 11:22 -0700, Andrew Thomas Pinski wrote:
> You cannot include c-common.h in the cfgexpand.c as some languages
> don't implement any thing in c-common.h.
Couldn't we move the wchar defines to tree.h too?
Then we wouldn't need to include the c-common.h file.
But I'm really not sure if that's the right way because I don't now how
the integer_type_kind enum is used. See attached patch.
Any other suggestions?
[-- Attachment #2: ssp.patch --]
[-- Type: text/x-patch, Size: 2167 bytes --]
Index: tree.h
===================================================================
--- tree.h (revision 140637)
+++ tree.h (working copy)
@@ -3792,6 +3792,9 @@
itk_unsigned_short,
itk_int,
itk_unsigned_int,
+ itk_wchar,
+ itk_signed_wchar,
+ itk_unsigned_wchar,
itk_long,
itk_unsigned_long,
itk_long_long,
@@ -3812,6 +3815,9 @@
#define short_unsigned_type_node integer_types[itk_unsigned_short]
#define integer_type_node integer_types[itk_int]
#define unsigned_type_node integer_types[itk_unsigned_int]
+#define wchar_type_node integer_types[itk_wchar]
+#define signed_wchar_type_node integer_types[itk_signed_wchar]
+#define unsigned_wchar_type_node integer_types[itk_unsigned_wchar]
#define long_integer_type_node integer_types[itk_long]
#define long_unsigned_type_node integer_types[itk_unsigned_long]
#define long_long_integer_type_node integer_types[itk_long_long]
Index: cfgexpand.c
===================================================================
--- cfgexpand.c (revision 140637)
+++ cfgexpand.c (working copy)
@@ -1201,7 +1201,10 @@
t = TYPE_MAIN_VARIANT (TREE_TYPE (type));
if (t == char_type_node
|| t == signed_char_type_node
- || t == unsigned_char_type_node)
+ || t == unsigned_char_type_node
+ || t == wchar_type_node
+ || t == signed_wchar_type_node
+ || t == unsigned_wchar_type_node)
{
unsigned HOST_WIDE_INT max = PARAM_VALUE (PARAM_SSP_BUFFER_SIZE);
unsigned HOST_WIDE_INT len;
Index: c-common.h
===================================================================
--- c-common.h (revision 140637)
+++ c-common.h (working copy)
@@ -235,9 +235,6 @@
#define char16_type_node c_global_trees[CTI_CHAR16_TYPE]
#define char32_type_node c_global_trees[CTI_CHAR32_TYPE]
-#define wchar_type_node c_global_trees[CTI_WCHAR_TYPE]
-#define signed_wchar_type_node c_global_trees[CTI_SIGNED_WCHAR_TYPE]
-#define unsigned_wchar_type_node c_global_trees[CTI_UNSIGNED_WCHAR_TYPE]
#define wint_type_node c_global_trees[CTI_WINT_TYPE]
#define signed_size_type_node c_global_trees[CTI_SIGNED_SIZE_TYPE]
#define unsigned_ptrdiff_type_node c_global_trees[CTI_UNSIGNED_PTRDIFF_TYPE]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
2008-09-25 15:40 ` Stefan Schulze Frielinghaus
@ 2008-09-25 22:51 ` Andrew Pinski
2008-09-26 18:06 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2008-09-25 22:51 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches
On Thu, Sep 25, 2008 at 7:58 AM, Stefan Schulze Frielinghaus
<stefan@seekline.net> wrote:
>
> On Wed, 2008-09-24 at 11:22 -0700, Andrew Thomas Pinski wrote:
>> You cannot include c-common.h in the cfgexpand.c as some languages
>> don't implement any thing in c-common.h.
>
> Couldn't we move the wchar defines to tree.h too?
> Then we wouldn't need to include the c-common.h file.
> But I'm really not sure if that's the right way because I don't now how
> the integer_type_kind enum is used. See attached patch.
Also I don't think this will work for C code where wchar_t is a
typedef. What about the new UTF-16 and UTF-16 types too?
So if I read your patch and the code correctly this is so
-fstack-protect does the same does the same for wchar_t and char
arrays, correct? I wonder why we make char arrays as special anyways.
Seems like any integer arrays should be handled the same way.
Thanks,
Andrew Pinski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
2008-09-25 22:51 ` Andrew Pinski
@ 2008-09-26 18:06 ` Stefan Schulze Frielinghaus
2008-10-19 5:51 ` Stefan Schulze Frielinghaus
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-09-26 18:06 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
On Thu, 2008-09-25 at 14:45 -0700, Andrew Pinski wrote:
> On Thu, Sep 25, 2008 at 7:58 AM, Stefan Schulze Frielinghaus
> <stefan@seekline.net> wrote:
> >
> > On Wed, 2008-09-24 at 11:22 -0700, Andrew Thomas Pinski wrote:
> >> You cannot include c-common.h in the cfgexpand.c as some languages
> >> don't implement any thing in c-common.h.
> >
> > Couldn't we move the wchar defines to tree.h too?
> > Then we wouldn't need to include the c-common.h file.
> > But I'm really not sure if that's the right way because I don't now how
> > the integer_type_kind enum is used. See attached patch.
>
> Also I don't think this will work for C code where wchar_t is a
> typedef. What about the new UTF-16 and UTF-16 types too?
Sure these one would need to be protected too. I'm note aware of how
they are constructed so I can't say that much about them. I will have a
look at them this weekend.
> So if I read your patch and the code correctly this is so
> -fstack-protect does the same does the same for wchar_t and char
> arrays, correct?
Yes. This was my intention.
> I wonder why we make char arrays as special anyways.
> Seems like any integer arrays should be handled the same way.
There was some discussion about this too. In the end the decision was to
only protect functions with character arrays. Because the overhead would
be to high to protect also integer arrays and so. I _guess_ that
statistically more character arrays (ak strings) overflow than
integer/float arrays. If you want to protect all functions (nevertheless
if they include an array of any type or not) use -fstack-protector-all
In the end the previous attached patch should solve the gap between
protecting functions with character arrays and wide character arrays.
cheers
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays
2008-09-26 18:06 ` Stefan Schulze Frielinghaus
@ 2008-10-19 5:51 ` Stefan Schulze Frielinghaus
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-10-19 5:51 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]
On Fri, 2008-09-26 at 19:34 +0200, Stefan Schulze Frielinghaus wrote:
> On Thu, 2008-09-25 at 14:45 -0700, Andrew Pinski wrote:
> > On Thu, Sep 25, 2008 at 7:58 AM, Stefan Schulze Frielinghaus
> > <stefan@seekline.net> wrote:
> > >
> > > On Wed, 2008-09-24 at 11:22 -0700, Andrew Thomas Pinski wrote:
> > >> You cannot include c-common.h in the cfgexpand.c as some languages
> > >> don't implement any thing in c-common.h.
> > >
> > > Couldn't we move the wchar defines to tree.h too?
> > > Then we wouldn't need to include the c-common.h file.
> > > But I'm really not sure if that's the right way because I don't now how
> > > the integer_type_kind enum is used. See attached patch.
> >
> > Also I don't think this will work for C code where wchar_t is a
> > typedef.
Yeah right. But SSP won't detect e.g. the following:
struct { int a, b, c; } buf;
strcpy ((char*)&buf, ">>>buffer overflow<<<");
It's just best practices. If you have a peace of code you don't trust
you may want to use -fstack-protector-all. SSP won't be perfect. But
with the attached patch there is some chance that GCC will detect some
arrays and protect them.
> What about the new UTF-16 and UTF-16 types too?
>
> Sure these one would need to be protected too. I'm note aware of how
> they are constructed so I can't say that much about them. I will have a
> look at them this weekend.
Like the same with wchar_t. See attached patch.
>
> > So if I read your patch and the code correctly this is so
> > -fstack-protect does the same does the same for wchar_t and char
> > arrays, correct?
>
> Yes. This was my intention.
>
> > I wonder why we make char arrays as special anyways.
> > Seems like any integer arrays should be handled the same way.
>
> There was some discussion about this too. In the end the decision was to
> only protect functions with character arrays. Because the overhead would
> be to high to protect also integer arrays and so. I _guess_ that
> statistically more character arrays (ak strings) overflow than
> integer/float arrays. If you want to protect all functions (nevertheless
> if they include an array of any type or not) use -fstack-protector-all
>
> In the end the previous attached patch should solve the gap between
> protecting functions with character arrays and wide character arrays.
>
> cheers
> Stefan
[-- Attachment #2: gcc.patch --]
[-- Type: text/x-patch, Size: 2644 bytes --]
Index: gcc/tree.h
===================================================================
--- gcc/tree.h (revision 141129)
+++ gcc/tree.h (working copy)
@@ -3793,8 +3793,13 @@
itk_char,
itk_signed_char,
itk_unsigned_char,
+ itk_char16,
itk_short,
itk_unsigned_short,
+ itk_wchar,
+ itk_signed_wchar,
+ itk_unsigned_wchar,
+ itk_char32,
itk_int,
itk_unsigned_int,
itk_long,
@@ -3813,8 +3818,13 @@
#define char_type_node integer_types[itk_char]
#define signed_char_type_node integer_types[itk_signed_char]
#define unsigned_char_type_node integer_types[itk_unsigned_char]
+#define char16_type_node integer_types[itk_char16]
#define short_integer_type_node integer_types[itk_short]
#define short_unsigned_type_node integer_types[itk_unsigned_short]
+#define wchar_type_node integer_types[itk_wchar]
+#define signed_wchar_type_node integer_types[itk_signed_wchar]
+#define unsigned_wchar_type_node integer_types[itk_unsigned_wchar]
+#define char32_type_node integer_types[itk_char32]
#define integer_type_node integer_types[itk_int]
#define unsigned_type_node integer_types[itk_unsigned_int]
#define long_integer_type_node integer_types[itk_long]
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c (revision 141129)
+++ gcc/cfgexpand.c (working copy)
@@ -1201,7 +1201,12 @@
t = TYPE_MAIN_VARIANT (TREE_TYPE (type));
if (t == char_type_node
|| t == signed_char_type_node
- || t == unsigned_char_type_node)
+ || t == unsigned_char_type_node
+ || t == wchar_type_node
+ || t == signed_wchar_type_node
+ || t == unsigned_wchar_type_node
+ || t == char16_type_node
+ || t == char32_type_node)
{
unsigned HOST_WIDE_INT max = PARAM_VALUE (PARAM_SSP_BUFFER_SIZE);
unsigned HOST_WIDE_INT len;
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h (revision 141129)
+++ gcc/c-common.h (working copy)
@@ -233,11 +233,6 @@
/* The number of items in the reserved keyword table. */
extern const unsigned int num_c_common_reswords;
-#define char16_type_node c_global_trees[CTI_CHAR16_TYPE]
-#define char32_type_node c_global_trees[CTI_CHAR32_TYPE]
-#define wchar_type_node c_global_trees[CTI_WCHAR_TYPE]
-#define signed_wchar_type_node c_global_trees[CTI_SIGNED_WCHAR_TYPE]
-#define unsigned_wchar_type_node c_global_trees[CTI_UNSIGNED_WCHAR_TYPE]
#define wint_type_node c_global_trees[CTI_WINT_TYPE]
#define signed_size_type_node c_global_trees[CTI_SIGNED_SIZE_TYPE]
#define unsigned_ptrdiff_type_node c_global_trees[CTI_UNSIGNED_PTRDIFF_TYPE]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-18 18:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-24 18:53 [PATCH] Fix Stack Smashing Protector to protect functions with wchar_t arrays Stefan Schulze Frielinghaus
2008-09-24 19:20 ` Andrew Thomas Pinski
2008-09-25 15:40 ` Stefan Schulze Frielinghaus
2008-09-25 22:51 ` Andrew Pinski
2008-09-26 18:06 ` Stefan Schulze Frielinghaus
2008-10-19 5:51 ` Stefan Schulze Frielinghaus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).