Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 14 Apr 2017 17:40:37 +0200
From: Florian Weimer <fw@...eb.enyo.de>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: oss-security@...ts.openwall.com
Subject: Re: alloca in inline functions can be dangerous

* Jason A. Donenfeld:

> Hey folks,
>
> I'm not sure this is the right mailing list to discuss this matter, but
> hopefully it finds an audience here. I was debugging some code recently,
> when I found a very nasty interaction between alloca and inline functions.
> Observe the following block:
>
> static inline void process_widget(struct widget *widget,
> 				  unsigned int  fcount)
> {
> 	struct fragment fragments[fcount];
> 	widgets_to_fragments(fragments, widget);
> 	process_fragments(fragments);
> }
>
> static void iterate_widgets(struct widgetlist *widgetlist)
> {
> 	struct widget *widget;
> 	unsigned int fcount;
>
> 	for (widget = widgetlist->first; widget; widget = widget->next) {
> 		fcount = widget_get_frags_required(widget);
> 		if (fcount > 256)
> 			continue;
> 		process_widget(widget, fcount);
> 	}
> }
>
> This seems pretty benign. However, let's look at two transformations that
> gcc makes. First the VLA is changed to use alloca:

Which GCC version do you use?

I turned your example into something that actually compiles:

struct widget
{
  struct widget *next;
};

unsigned widget_get_frags_required(struct widget *);

struct fragment { int dummy; };
void widgets_to_fragments(struct fragment *, struct widget *);
void process_fragments(struct fragment *);

struct widgetlist
{
  struct widget *first;
};

static inline void
process_widget(struct widget *widget, unsigned int  fcount)
{
  struct fragment fragments[fcount];
  widgets_to_fragments(fragments, widget);
  process_fragments(fragments);
}

void
iterate_widgets(struct widgetlist *widgetlist)
{
  struct widget *widget;
  unsigned int fcount;

  for (widget = widgetlist->first; widget; widget = widget->next) {
    fcount = widget_get_frags_required(widget);
    if (fcount > 256)
      continue;
    process_widget(widget, fcount);
  }
}

I don't see the behavior your outline.  The stack pointer is restored
for each iteration of the loop.

In my experience, GCC is pretty good at actually deallocating VLAs
when a scope is exited.

In any case, this is a GCC bug.  I can help you to turn this into a
proper bug report if it still exists in current versions.

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ