public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Unnecessary overhead with stack protector.
@ 2009-10-15 18:35 Dave Jones
  2009-10-15 19:07 ` Ingo Molnar
  2009-10-22  1:26 ` Unnecessary overhead with stack protector Andrew Morton
  0 siblings, 2 replies; 16+ messages in thread
From: Dave Jones @ 2009-10-15 18:35 UTC (permalink / raw)
  To: Linux Kernel; +Cc: Ingo Molnar, Thomas Gleixner, esandeen, cebbert

113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
made CC_STACKPROTECTOR_ALL not-selectable if someone enables CC_STACKPROTECTOR.

We've noticed in Fedora that this has introduced noticable overhead on
some functions, including those which don't even have any on-stack variables.

According to the gcc manpage, -fstack-protector will protect functions with
as little as 8 bytes of stack usage. So we're introducing a huge amount
of overhead, to close a small amount of vulnerability (the >0 && <8 case).

The overhead as it stands right now means this whole option is unusable for
a distro kernel without reverting the above commit.

	Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-15 18:35 Unnecessary overhead with stack protector Dave Jones
@ 2009-10-15 19:07 ` Ingo Molnar
  2009-10-21 15:50   ` Eric Sandeen
  2009-10-22  1:26 ` Unnecessary overhead with stack protector Andrew Morton
  1 sibling, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2009-10-15 19:07 UTC (permalink / raw)
  To: Dave Jones, Linux Kernel, Thomas Gleixner, esandeen, cebbert,
	Arjan van de Ven, H. Peter Anvin


(Cc:-ed Arjan too.)

* Dave Jones <davej@redhat.com> wrote:

> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that made 
> CC_STACKPROTECTOR_ALL not-selectable if someone enables 
> CC_STACKPROTECTOR.
> 
> We've noticed in Fedora that this has introduced noticable overhead on 
> some functions, including those which don't even have any on-stack 
> variables.
> 
> According to the gcc manpage, -fstack-protector will protect functions 
> with as little as 8 bytes of stack usage. So we're introducing a huge 
> amount of overhead, to close a small amount of vulnerability (the >0 
> && <8 case).
> 
> The overhead as it stands right now means this whole option is 
> unusable for a distro kernel without reverting the above commit.

Exactly what workload showed overhead, and how much?

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-15 19:07 ` Ingo Molnar
@ 2009-10-21 15:50   ` Eric Sandeen
  2009-10-21 18:00     ` Arjan van de Ven
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Sandeen @ 2009-10-21 15:50 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Dave Jones, Linux Kernel, Thomas Gleixner, esandeen, cebbert,
	Arjan van de Ven, H. Peter Anvin

Ingo Molnar wrote:
> (Cc:-ed Arjan too.)
> 
> * Dave Jones <davej@redhat.com> wrote:
> 
>> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that made 
>> CC_STACKPROTECTOR_ALL not-selectable if someone enables 
>> CC_STACKPROTECTOR.
>>
>> We've noticed in Fedora that this has introduced noticable overhead on 
>> some functions, including those which don't even have any on-stack 
>> variables.
>>
>> According to the gcc manpage, -fstack-protector will protect functions 
>> with as little as 8 bytes of stack usage. So we're introducing a huge 
>> amount of overhead, to close a small amount of vulnerability (the >0 
>> && <8 case).
>>
>> The overhead as it stands right now means this whole option is 
>> unusable for a distro kernel without reverting the above commit.
> 
> Exactly what workload showed overhead, and how much?
> 
> 	Ingo

I had xfs blowing up pretty nicely; granted, xfs is not svelte but it
was never this bad before.

-Eric


         Depth    Size   Location    (65 entries)
         -----    ----   --------
   0)     7280      80   check_object+0x6c/0x1d3
   1)     7200     112   __slab_alloc+0x332/0x3f0
   2)     7088      16   kmem_cache_alloc+0xcb/0x18a
   3)     7072     112   mempool_alloc_slab+0x28/0x3e
   4)     6960     128   mempool_alloc+0x71/0x13c
   5)     6832      32   scsi_sg_alloc+0x5d/0x73
   6)     6800     128   __sg_alloc_table+0x6f/0x134
   7)     6672      64   scsi_alloc_sgtable+0x3b/0x74
   8)     6608      48   scsi_init_sgtable+0x34/0x8c
   9)     6560      80   scsi_init_io+0x3e/0x177
  10)     6480      48   scsi_setup_fs_cmnd+0x9c/0xb9
  11)     6432     160   sd_prep_fn+0x69/0x8bd
  12)     6272      64   blk_peek_request+0xf0/0x1c8
  13)     6208     112   scsi_request_fn+0x92/0x4c4
  14)     6096      48   __blk_run_queue+0x54/0x9a
  15)     6048      80   elv_insert+0xbd/0x1e0
  16)     5968      64   __elv_add_request+0xa7/0xc2
  17)     5904      64   blk_insert_cloned_request+0x90/0xc8
  18)     5840      48   dm_dispatch_request+0x4f/0x8b
  19)     5792      96   dm_request_fn+0x141/0x1ca
  20)     5696      48   __blk_run_queue+0x54/0x9a
  21)     5648      80   cfq_insert_request+0x39d/0x3d4
  22)     5568      80   elv_insert+0x120/0x1e0
  23)     5488      64   __elv_add_request+0xa7/0xc2
  24)     5424      96   __make_request+0x35e/0x3f1
  25)     5328      64   dm_request+0x55/0x234
  26)     5264     128   generic_make_request+0x29e/0x2fc
  27)     5136      80   submit_bio+0xe3/0x100
  28)     5056     112   _xfs_buf_ioapply+0x21d/0x25c [xfs]
  29)     4944      48   xfs_buf_iorequest+0x58/0x9f [xfs]
  30)     4896      48   _xfs_buf_read+0x45/0x74 [xfs]
  31)     4848      48   xfs_buf_read_flags+0x67/0xb5 [xfs]
  32)     4800     112   xfs_trans_read_buf+0x1be/0x2c2 [xfs]
  33)     4688     112   xfs_btree_read_buf_block+0x64/0xbc [xfs]
  34)     4576      96   xfs_btree_lookup_get_block+0x9c/0xd8 [xfs]
  35)     4480     192   xfs_btree_lookup+0x14a/0x408 [xfs]
  36)     4288      32   xfs_alloc_lookup_eq+0x2c/0x42 [xfs]
  37)     4256     112   xfs_alloc_fixup_trees+0x85/0x2b4 [xfs]
  38)     4144     176   xfs_alloc_ag_vextent_near+0x339/0x8e8 [xfs]
  39)     3968      48   xfs_alloc_ag_vextent+0x44/0x126 [xfs]
  40)     3920     128   xfs_alloc_vextent+0x2b1/0x403 [xfs]
  41)     3792     272   xfs_bmap_btalloc+0x4fc/0x6d4 [xfs]
  42)     3520      32   xfs_bmap_alloc+0x21/0x37 [xfs]
  43)     3488     464   xfs_bmapi+0x70b/0xde1 [xfs]
  44)     3024     256   xfs_iomap_write_allocate+0x21d/0x35d [xfs]
  45)     2768     192   xfs_iomap+0x208/0x28a [xfs]
  46)     2576      48   xfs_map_blocks+0x3d/0x5a [xfs]
  47)     2528     256   xfs_page_state_convert+0x2b8/0x589 [xfs]
  48)     2272      96   xfs_vm_writepage+0xbf/0x10e [xfs]
  49)     2176      48   __writepage+0x29/0x5f
  50)     2128     320   write_cache_pages+0x27b/0x415
  51)     1808      32   generic_writepages+0x38/0x4e
  52)     1776      80   xfs_vm_writepages+0x60/0x7f [xfs]
  53)     1696      48   do_writepages+0x3d/0x63
  54)     1648     144   writeback_single_inode+0x169/0x29d
  55)     1504     112   generic_sync_sb_inodes+0x21d/0x37f
  56)     1392      64   writeback_inodes+0xb6/0x125
  57)     1328     192   balance_dirty_pages_ratelimited_nr+0x172/0x2b0
  58)     1136     240   generic_file_buffered_write+0x240/0x33c
  59)      896     256   xfs_write+0x4d4/0x723 [xfs]
  60)      640      32   xfs_file_aio_write+0x79/0x8f [xfs]
  61)      608     320   do_sync_write+0xfa/0x14b
  62)      288      80   vfs_write+0xbd/0x12e
  63)      208      80   sys_write+0x59/0x91
  64)      128     128   system_call_fastpath+0x16/0x1b

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-21 15:50   ` Eric Sandeen
@ 2009-10-21 18:00     ` Arjan van de Ven
  2009-10-21 18:59       ` Eric Sandeen
  0 siblings, 1 reply; 16+ messages in thread
From: Arjan van de Ven @ 2009-10-21 18:00 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Ingo Molnar, Dave Jones, Linux Kernel, Thomas Gleixner, esandeen,
	cebbert, H. Peter Anvin

On Wed, 21 Oct 2009 10:50:02 -0500
Eric Sandeen <sandeen@redhat.com> wrote:

> Ingo Molnar wrote:
> > (Cc:-ed Arjan too.)
> > 
> > * Dave Jones <davej@redhat.com> wrote:
> > 
> >> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
> >> made CC_STACKPROTECTOR_ALL not-selectable if someone enables 
> >> CC_STACKPROTECTOR.
> >>
> >> We've noticed in Fedora that this has introduced noticable
> >> overhead on some functions, including those which don't even have
> >> any on-stack variables.
> >>
> >> According to the gcc manpage, -fstack-protector will protect
> >> functions with as little as 8 bytes of stack usage. So we're
> >> introducing a huge amount of overhead, to close a small amount of
> >> vulnerability (the >0 && <8 case).
> >>
> >> The overhead as it stands right now means this whole option is 
> >> unusable for a distro kernel without reverting the above commit.
> > 
> > Exactly what workload showed overhead, and how much?
> > 
> > 	Ingo
> 
> I had xfs blowing up pretty nicely; granted, xfs is not svelte but it
> was never this bad before.
> 

do you have any indication that SP actually increases the stack
footprint by that much? it's only a few bytes....


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-21 18:00     ` Arjan van de Ven
@ 2009-10-21 18:59       ` Eric Sandeen
  2009-10-21 19:09         ` Eric Sandeen
  2009-10-21 19:16         ` XFS stack overhead Ingo Molnar
  0 siblings, 2 replies; 16+ messages in thread
From: Eric Sandeen @ 2009-10-21 18:59 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Ingo Molnar, Dave Jones, Linux Kernel, Thomas Gleixner, esandeen,
	cebbert, H. Peter Anvin

Arjan van de Ven wrote:
> On Wed, 21 Oct 2009 10:50:02 -0500
> Eric Sandeen <sandeen@redhat.com> wrote:
> 
>> Ingo Molnar wrote:
>>> (Cc:-ed Arjan too.)
>>>
>>> * Dave Jones <davej@redhat.com> wrote:
>>>
>>>> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
>>>> made CC_STACKPROTECTOR_ALL not-selectable if someone enables 
>>>> CC_STACKPROTECTOR.
>>>>
>>>> We've noticed in Fedora that this has introduced noticable
>>>> overhead on some functions, including those which don't even have
>>>> any on-stack variables.
>>>>
>>>> According to the gcc manpage, -fstack-protector will protect
>>>> functions with as little as 8 bytes of stack usage. So we're
>>>> introducing a huge amount of overhead, to close a small amount of
>>>> vulnerability (the >0 && <8 case).
>>>>
>>>> The overhead as it stands right now means this whole option is 
>>>> unusable for a distro kernel without reverting the above commit.
>>> Exactly what workload showed overhead, and how much?
>>>
>>> 	Ingo
>> I had xfs blowing up pretty nicely; granted, xfs is not svelte but it
>> was never this bad before.
>>
> 
> do you have any indication that SP actually increases the stack
> footprint by that much? it's only a few bytes....
> 
> 

Here's a sample of some of the largest xfs stack users,
and the effect stack-protector had on them.  This was just
done with objdump -d xfs.ko | scripts/checkstack.pl; I don't
know if there's extra runtime stack overhead w/ stackprotector?

-Eric

function                  nostack stackprot delta delta %
xfs_bmapi                      376      408    32  9%
xfs_bulkstat                   328      344    16  5%
_xfs_trans_commit              296      312    16  5%
xfs_iomap_write_delay          264      280    16  6%
xfs_file_ioctl                 248      312    64 26%
xfs_symlink                    248      264    16  6%
xfs_bunmapi                    232      280    48 21%
xlog_do_recovery_pass          232      248    16  7%
xfs_trans_unreserve_and_mod_sb 224      240    16  7%
xfs_bmap_del_extent            216      248    32 15%
xfs_cluster_write              216      232    16  7%
xfs_file_compat_ioctl          216      296    80 37%
xfs_attr_set_int               200      216    16  8%
xfs_bmap_add_extent_delay_real 200      248    48 24%


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-21 18:59       ` Eric Sandeen
@ 2009-10-21 19:09         ` Eric Sandeen
  2009-10-21 19:24           ` Eric Sandeen
  2009-10-21 19:16         ` XFS stack overhead Ingo Molnar
  1 sibling, 1 reply; 16+ messages in thread
From: Eric Sandeen @ 2009-10-21 19:09 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Ingo Molnar, Dave Jones, Linux Kernel, Thomas Gleixner, esandeen,
	cebbert, H. Peter Anvin

Eric Sandeen wrote:
> Arjan van de Ven wrote:

...

>> do you have any indication that SP actually increases the stack
>> footprint by that much? it's only a few bytes....
>>
>>
> 
> Here's a sample of some of the largest xfs stack users,
> and the effect stack-protector had on them.  This was just
> done with objdump -d xfs.ko | scripts/checkstack.pl; I don't
> know if there's extra runtime stack overhead w/ stackprotector?
> 
> -Eric
> 
> function                  nostack stackprot delta delta %
> xfs_bmapi                      376      408    32  9%
> xfs_bulkstat                   328      344    16  5%
> _xfs_trans_commit              296      312    16  5%
> xfs_iomap_write_delay          264      280    16  6%
> xfs_file_ioctl                 248      312    64 26%
> xfs_symlink                    248      264    16  6%
> xfs_bunmapi                    232      280    48 21%
> xlog_do_recovery_pass          232      248    16  7%
> xfs_trans_unreserve_and_mod_sb 224      240    16  7%
> xfs_bmap_del_extent            216      248    32 15%
> xfs_cluster_write              216      232    16  7%
> xfs_file_compat_ioctl          216      296    80 37%
> xfs_attr_set_int               200      216    16  8%
> xfs_bmap_add_extent_delay_real 200      248    48 24%
> 

but maybe more to Dave's original point, xfs on x86_64 in my tree had
243 functions with minimal stack usage of 8 bytes.  w/
CC_STACKPROTECTOR_ALL in force, I end up with these sizes for those
functions:

  count bytes
      3 16
    236 24
      1 32
      5 40

8->24 bytes is pretty significant too, w/ a 200% increase, if you add a
few up...

-Eric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: XFS stack overhead
  2009-10-21 18:59       ` Eric Sandeen
  2009-10-21 19:09         ` Eric Sandeen
@ 2009-10-21 19:16         ` Ingo Molnar
  2009-10-21 19:21           ` Eric Sandeen
  1 sibling, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2009-10-21 19:16 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Arjan van de Ven, Dave Jones, Linux Kernel, Thomas Gleixner,
	esandeen, cebbert, H. Peter Anvin


* Eric Sandeen <sandeen@redhat.com> wrote:

> Arjan van de Ven wrote:
> > On Wed, 21 Oct 2009 10:50:02 -0500
> > Eric Sandeen <sandeen@redhat.com> wrote:
> > 
> >> Ingo Molnar wrote:
> >>> (Cc:-ed Arjan too.)
> >>>
> >>> * Dave Jones <davej@redhat.com> wrote:
> >>>
> >>>> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
> >>>> made CC_STACKPROTECTOR_ALL not-selectable if someone enables 
> >>>> CC_STACKPROTECTOR.
> >>>>
> >>>> We've noticed in Fedora that this has introduced noticable
> >>>> overhead on some functions, including those which don't even have
> >>>> any on-stack variables.
> >>>>
> >>>> According to the gcc manpage, -fstack-protector will protect
> >>>> functions with as little as 8 bytes of stack usage. So we're
> >>>> introducing a huge amount of overhead, to close a small amount of
> >>>> vulnerability (the >0 && <8 case).
> >>>>
> >>>> The overhead as it stands right now means this whole option is 
> >>>> unusable for a distro kernel without reverting the above commit.
> >>> Exactly what workload showed overhead, and how much?
> >>>
> >>> 	Ingo
> >> I had xfs blowing up pretty nicely; granted, xfs is not svelte but it
> >> was never this bad before.
> >>
> > 
> > do you have any indication that SP actually increases the stack
> > footprint by that much? it's only a few bytes....
> > 
> > 
> 
> Here's a sample of some of the largest xfs stack users,
> and the effect stack-protector had on them.  This was just
> done with objdump -d xfs.ko | scripts/checkstack.pl; I don't
> know if there's extra runtime stack overhead w/ stackprotector?
> 
> -Eric
> 
> function                  nostack stackprot delta delta %
> xfs_bmapi                      376      408    32  9%
> xfs_bulkstat                   328      344    16  5%
> _xfs_trans_commit              296      312    16  5%
> xfs_iomap_write_delay          264      280    16  6%
> xfs_file_ioctl                 248      312    64 26%
> xfs_symlink                    248      264    16  6%
> xfs_bunmapi                    232      280    48 21%
> xlog_do_recovery_pass          232      248    16  7%
> xfs_trans_unreserve_and_mod_sb 224      240    16  7%
> xfs_bmap_del_extent            216      248    32 15%
> xfs_cluster_write              216      232    16  7%
> xfs_file_compat_ioctl          216      296    80 37%
> xfs_attr_set_int               200      216    16  8%
> xfs_bmap_add_extent_delay_real 200      248    48 24%

Note that those are very large stack frames to begin with.

3496 bytes - that's a _lot_ - can anyone even run XFS with 4K stacks on? 

With stackprotector it's 3928 - a 12% increase - which certainly does 
not help - but the basic problem is the large stack footprint to begin 
with.

Also, the posting apparently mixes 'stack overhead' with 'runtime 
overhead'.

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: XFS stack overhead
  2009-10-21 19:16         ` XFS stack overhead Ingo Molnar
@ 2009-10-21 19:21           ` Eric Sandeen
  2009-10-21 20:22             ` Chuck Ebbert
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Sandeen @ 2009-10-21 19:21 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arjan van de Ven, Dave Jones, Linux Kernel, Thomas Gleixner,
	esandeen, cebbert, H. Peter Anvin

Ingo Molnar wrote:
> * Eric Sandeen <sandeen@redhat.com> wrote:
> 
>> Arjan van de Ven wrote:
>>> On Wed, 21 Oct 2009 10:50:02 -0500
>>> Eric Sandeen <sandeen@redhat.com> wrote:
>>>
>>>> Ingo Molnar wrote:
>>>>> (Cc:-ed Arjan too.)
>>>>>
>>>>> * Dave Jones <davej@redhat.com> wrote:
>>>>>
>>>>>> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
>>>>>> made CC_STACKPROTECTOR_ALL not-selectable if someone enables 
>>>>>> CC_STACKPROTECTOR.
>>>>>>
>>>>>> We've noticed in Fedora that this has introduced noticable
>>>>>> overhead on some functions, including those which don't even have
>>>>>> any on-stack variables.
>>>>>>
>>>>>> According to the gcc manpage, -fstack-protector will protect
>>>>>> functions with as little as 8 bytes of stack usage. So we're
>>>>>> introducing a huge amount of overhead, to close a small amount of
>>>>>> vulnerability (the >0 && <8 case).
>>>>>>
>>>>>> The overhead as it stands right now means this whole option is 
>>>>>> unusable for a distro kernel without reverting the above commit.
>>>>> Exactly what workload showed overhead, and how much?
>>>>>
>>>>> 	Ingo
>>>> I had xfs blowing up pretty nicely; granted, xfs is not svelte but it
>>>> was never this bad before.
>>>>
>>> do you have any indication that SP actually increases the stack
>>> footprint by that much? it's only a few bytes....
>>>
>>>
>> Here's a sample of some of the largest xfs stack users,
>> and the effect stack-protector had on them.  This was just
>> done with objdump -d xfs.ko | scripts/checkstack.pl; I don't
>> know if there's extra runtime stack overhead w/ stackprotector?
>>
>> -Eric
>>
>> function                  nostack stackprot delta delta %
>> xfs_bmapi                      376      408    32  9%
>> xfs_bulkstat                   328      344    16  5%
>> _xfs_trans_commit              296      312    16  5%
>> xfs_iomap_write_delay          264      280    16  6%
>> xfs_file_ioctl                 248      312    64 26%
>> xfs_symlink                    248      264    16  6%
>> xfs_bunmapi                    232      280    48 21%
>> xlog_do_recovery_pass          232      248    16  7%
>> xfs_trans_unreserve_and_mod_sb 224      240    16  7%
>> xfs_bmap_del_extent            216      248    32 15%
>> xfs_cluster_write              216      232    16  7%
>> xfs_file_compat_ioctl          216      296    80 37%
>> xfs_attr_set_int               200      216    16  8%
>> xfs_bmap_add_extent_delay_real 200      248    48 24%
> 
> Note that those are very large stack frames to begin with.
> 
> 3496 bytes - that's a _lot_ - can anyone even run XFS with 4K stacks on? 

The above isn't a callchain; those are just the biggest users.

Yes, xfs works w/4k stacks but sometimes not over complex storage.

> With stackprotector it's 3928 - a 12% increase - which certainly does 
> not help - but the basic problem is the large stack footprint to begin 
> with.

I can find plenty of examples of > 300 bytes stack users in the core
kernel write path too, I'm just using xfs as an example...

> Also, the posting apparently mixes 'stack overhead' with 'runtime 
> overhead'.

right, that's why I asked, I'm not sure if stackprotector has runtime
overhead as well.

-Eric

> 	Ingo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-21 19:09         ` Eric Sandeen
@ 2009-10-21 19:24           ` Eric Sandeen
  2009-10-21 21:08             ` Chuck Ebbert
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Sandeen @ 2009-10-21 19:24 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Ingo Molnar, Dave Jones, Linux Kernel, Thomas Gleixner, esandeen,
	cebbert, H. Peter Anvin

Eric Sandeen wrote:
> Eric Sandeen wrote:

..

> but maybe more to Dave's original point, xfs on x86_64 in my tree had
> 243 functions with minimal stack usage of 8 bytes.  w/
> CC_STACKPROTECTOR_ALL in force, I end up with these sizes for those
> functions:
> 
>   count bytes
>       3 16
>     236 24
>       1 32
>       5 40
> 
> 8->24 bytes is pretty significant too, w/ a 200% increase, if you add a
> few up...

And on top of that (sorry for the self-replies) there are 600+ functions
in xfs that didn't even register a stack footprint at all (i.e. no sub
%rsp in disassembly) w/o stackprotector, which now have 16 or 24 bytes
with it on.

Forgive me if I'm not using the right tools to look, but it seems to me
that in the aggregate, CC_STACKPROTECTOR_ALL has a pretty big impact.

-Eric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: XFS stack overhead
  2009-10-21 19:21           ` Eric Sandeen
@ 2009-10-21 20:22             ` Chuck Ebbert
  0 siblings, 0 replies; 16+ messages in thread
From: Chuck Ebbert @ 2009-10-21 20:22 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Ingo Molnar, Arjan van de Ven, Dave Jones, Linux Kernel,
	Thomas Gleixner, esandeen, H. Peter Anvin

On Wed, 21 Oct 2009 14:21:54 -0500
Eric Sandeen <sandeen@redhat.com> wrote:

> > Also, the posting apparently mixes 'stack overhead' with 'runtime 
> > overhead'.
> 
> right, that's why I asked, I'm not sure if stackprotector has runtime
> overhead as well.
> 

A bigger stack causes runtime overhead too because it increases the cache
footprint of the workload.

For the function I looked at the insn overhead was substantial:

On entry:
ffffffff8113ffa0:       48 83 ec 10             sub    $0x10,%rsp
...
ffffffff8113ffa9:       65 48 8b 04 25 28 00    mov    %gs:0x28,%rax
ffffffff8113ffb0:       00 00
ffffffff8113ffb2:       48 89 45 f8             mov    %rax,-0x8(%rbp)

On exit:
ffffffff81140000:       48 8b 55 f8             mov    -0x8(%rbp),%rdx
ffffffff81140004:       65 48 33 14 25 28 00    xor    %gs:0x28,%rdx
ffffffff8114000b:       00 00
ffffffff8114000d:       74 13                   je     ffffffff81140022 <mem_cgroup_get_reclaim_stat_from_page+0x86>
...
ffffffff8114001d:       e8 ef 42 f2 ff          callq  ffffffff81064311 <__stack_chk_fail>

So that's 37 extra bytes of code: 1 subtract, 4 reg/mem moves, an xor
and a conditional jump that always get executed -- on every function
call.

And all that, in this case, for a function that doesn't even have any
on-stack variables and can hardly be expected to be vulnerable to
stack-smashing attacks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-21 19:24           ` Eric Sandeen
@ 2009-10-21 21:08             ` Chuck Ebbert
  0 siblings, 0 replies; 16+ messages in thread
From: Chuck Ebbert @ 2009-10-21 21:08 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Arjan van de Ven, Ingo Molnar, Dave Jones, Linux Kernel,
	Thomas Gleixner, esandeen, H. Peter Anvin

On Wed, 21 Oct 2009 14:24:18 -0500
Eric Sandeen <sandeen@redhat.com> wrote:

> 
> Forgive me if I'm not using the right tools to look, but it seems to me
> that in the aggregate, CC_STACKPROTECTOR_ALL has a pretty big impact.

FWIW, I think we'll end up either disabling the stackprotector completely
in Fedora, or at least reverting the patch that adds all that overhead.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-15 18:35 Unnecessary overhead with stack protector Dave Jones
  2009-10-15 19:07 ` Ingo Molnar
@ 2009-10-22  1:26 ` Andrew Morton
  2009-10-26 16:30   ` Chuck Ebbert
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2009-10-22  1:26 UTC (permalink / raw)
  To: Dave Jones; +Cc: Linux Kernel, Ingo Molnar, Thomas Gleixner, esandeen, cebbert

On Thu, 15 Oct 2009 14:35:41 -0400 Dave Jones <davej@redhat.com> wrote:

> 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
> made CC_STACKPROTECTOR_ALL not-selectable if someone enables CC_STACKPROTECTOR.
> 
> We've noticed in Fedora that this has introduced noticable overhead on
> some functions, including those which don't even have any on-stack variables.
> 
> According to the gcc manpage, -fstack-protector will protect functions with
> as little as 8 bytes of stack usage. So we're introducing a huge amount
> of overhead, to close a small amount of vulnerability (the >0 && <8 case).
> 
> The overhead as it stands right now means this whole option is unusable for
> a distro kernel without reverting the above commit.
> 

This looks like a fairly serious problem to me, but I'm confused by the
commit ID.  February 2008 - is this correct?

If so, this seems like a rather long period of time in which to make such a
discovery.

Also, the Kconfig fiddle didn't cause the problem - it just revealed it. 
The core problem of increased stack usage and text size should already have
been known (to stackprotector developers, at least).  But it sounds like it
wasn't.

So perhaps this was all triggered by a particular gcc version?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-22  1:26 ` Unnecessary overhead with stack protector Andrew Morton
@ 2009-10-26 16:30   ` Chuck Ebbert
  2009-10-26 16:37     ` Andrew Morton
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Ebbert @ 2009-10-26 16:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dave Jones, Linux Kernel, Ingo Molnar, Thomas Gleixner, esandeen

On Wed, 21 Oct 2009 18:26:36 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 15 Oct 2009 14:35:41 -0400 Dave Jones <davej@redhat.com> wrote:
> 
> > 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
> > made CC_STACKPROTECTOR_ALL not-selectable if someone enables CC_STACKPROTECTOR.
> > 
> > We've noticed in Fedora that this has introduced noticable overhead on
> > some functions, including those which don't even have any on-stack variables.
> > 
> > According to the gcc manpage, -fstack-protector will protect functions with
> > as little as 8 bytes of stack usage. So we're introducing a huge amount
> > of overhead, to close a small amount of vulnerability (the >0 && <8 case).
> > 
> > The overhead as it stands right now means this whole option is unusable for
> > a distro kernel without reverting the above commit.
> > 
> 
> This looks like a fairly serious problem to me, but I'm confused by the
> commit ID.  February 2008 - is this correct?
> 

That date is pure fiction AFAICT. And the Mercurial kernel repo says May 2008...
Is there some way to get the date a change was merged into the official tree as
opposed to the date it was created in some other tree?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-26 16:30   ` Chuck Ebbert
@ 2009-10-26 16:37     ` Andrew Morton
  2009-10-26 16:56       ` Chuck Ebbert
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2009-10-26 16:37 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Dave Jones, Linux Kernel, Ingo Molnar, Thomas Gleixner, esandeen

On Mon, 26 Oct 2009 12:30:04 -0400 Chuck Ebbert <cebbert@redhat.com> wrote:

> On Wed, 21 Oct 2009 18:26:36 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 15 Oct 2009 14:35:41 -0400 Dave Jones <davej@redhat.com> wrote:
> > 
> > > 113c5413cf9051cc50b88befdc42e3402bb92115 introduced a change that
> > > made CC_STACKPROTECTOR_ALL not-selectable if someone enables CC_STACKPROTECTOR.
> > > 
> > > We've noticed in Fedora that this has introduced noticable overhead on
> > > some functions, including those which don't even have any on-stack variables.
> > > 
> > > According to the gcc manpage, -fstack-protector will protect functions with
> > > as little as 8 bytes of stack usage. So we're introducing a huge amount
> > > of overhead, to close a small amount of vulnerability (the >0 && <8 case).
> > > 
> > > The overhead as it stands right now means this whole option is unusable for
> > > a distro kernel without reverting the above commit.
> > > 
> > 
> > This looks like a fairly serious problem to me, but I'm confused by the
> > commit ID.  February 2008 - is this correct?
> > 
> 
> That date is pure fiction AFAICT. And the Mercurial kernel repo says May 2008...
> Is there some way to get the date a change was merged into the official tree as
> opposed to the date it was created in some other tree?

oh, so someone _did_ read my email!


git show --pretty=fuller 113c5413cf9051cc50b88befdc42e3402bb92115

commit 113c5413cf9051cc50b88befdc42e3402bb92115
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Thu Feb 14 10:36:03 2008 +0100
Commit:     Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon May 26 16:15:32 2008 +0200

I think the CommitDate is when it hit mainline.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-26 16:37     ` Andrew Morton
@ 2009-10-26 16:56       ` Chuck Ebbert
  2009-10-26 20:03         ` Ingo Molnar
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Ebbert @ 2009-10-26 16:56 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Dave Jones, Linux Kernel, Ingo Molnar, Thomas Gleixner, esandeen

On Mon, 26 Oct 2009 09:37:06 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> > > This looks like a fairly serious problem to me, but I'm confused by the
> > > commit ID.  February 2008 - is this correct?
> > > 
> > 
> > That date is pure fiction AFAICT. And the Mercurial kernel repo says May 2008...
> > Is there some way to get the date a change was merged into the official tree as
> > opposed to the date it was created in some other tree?
> 
> oh, so someone _did_ read my email!
> 
> 
> git show --pretty=fuller 113c5413cf9051cc50b88befdc42e3402bb92115
> 
> commit 113c5413cf9051cc50b88befdc42e3402bb92115
> Author:     Ingo Molnar <mingo@elte.hu>
> AuthorDate: Thu Feb 14 10:36:03 2008 +0100
> Commit:     Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Mon May 26 16:15:32 2008 +0200
> 
> I think the CommitDate is when it hit mainline.

The X-Git-Tag from the web page says it first appeared in mainline in
2.6.30-rc1:

From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 14 Feb 2008 09:36:03 +0000 (+0100)
Subject: x86: unify stackprotector features
X-Git-Tag: v2.6.30-rc1~2^2~50^2~67^2~11
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=113c5413c

Looks like it went through three merges on the way there?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Unnecessary overhead with stack protector.
  2009-10-26 16:56       ` Chuck Ebbert
@ 2009-10-26 20:03         ` Ingo Molnar
  0 siblings, 0 replies; 16+ messages in thread
From: Ingo Molnar @ 2009-10-26 20:03 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Andrew Morton, Dave Jones, Linux Kernel, Thomas Gleixner,
	esandeen


* Chuck Ebbert <cebbert@redhat.com> wrote:

> On Mon, 26 Oct 2009 09:37:06 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > > This looks like a fairly serious problem to me, but I'm confused by the
> > > > commit ID.  February 2008 - is this correct?
> > > > 
> > > 
> > > That date is pure fiction AFAICT. And the Mercurial kernel repo says May 2008...
> > > Is there some way to get the date a change was merged into the official tree as
> > > opposed to the date it was created in some other tree?
> > 
> > oh, so someone _did_ read my email!
> > 
> > 
> > git show --pretty=fuller 113c5413cf9051cc50b88befdc42e3402bb92115
> > 
> > commit 113c5413cf9051cc50b88befdc42e3402bb92115
> > Author:     Ingo Molnar <mingo@elte.hu>
> > AuthorDate: Thu Feb 14 10:36:03 2008 +0100
> > Commit:     Thomas Gleixner <tglx@linutronix.de>
> > CommitDate: Mon May 26 16:15:32 2008 +0200
> > 
> > I think the CommitDate is when it hit mainline.
> 
> The X-Git-Tag from the web page says it first appeared in mainline in
> 2.6.30-rc1:
> 
> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu, 14 Feb 2008 09:36:03 +0000 (+0100)
> Subject: x86: unify stackprotector features
> X-Git-Tag: v2.6.30-rc1~2^2~50^2~67^2~11
> X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=113c5413c
> 
> Looks like it went through three merges on the way there?

yep. Note that x86/urgent has this queued up now:

 14a3f40: x86: Remove STACKPROTECTOR_ALL

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-10-26 20:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-15 18:35 Unnecessary overhead with stack protector Dave Jones
2009-10-15 19:07 ` Ingo Molnar
2009-10-21 15:50   ` Eric Sandeen
2009-10-21 18:00     ` Arjan van de Ven
2009-10-21 18:59       ` Eric Sandeen
2009-10-21 19:09         ` Eric Sandeen
2009-10-21 19:24           ` Eric Sandeen
2009-10-21 21:08             ` Chuck Ebbert
2009-10-21 19:16         ` XFS stack overhead Ingo Molnar
2009-10-21 19:21           ` Eric Sandeen
2009-10-21 20:22             ` Chuck Ebbert
2009-10-22  1:26 ` Unnecessary overhead with stack protector Andrew Morton
2009-10-26 16:30   ` Chuck Ebbert
2009-10-26 16:37     ` Andrew Morton
2009-10-26 16:56       ` Chuck Ebbert
2009-10-26 20:03         ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox