oss-sec mailing list archives

Re: alloca in inline functions can be dangerous


From: Florian Weimer <fw () deneb enyo de>
Date: Fri, 14 Apr 2017 17:40:37 +0200

* 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.


Current thread: