Bug 209232

Summary: kernel doesn't honour executable PT_GNU_STACK
Product: [Fedora] Fedora Reporter: David Woodhouse <dwmw2>
Component: kernelAssignee: Dave Jones <davej>
Status: CLOSED RAWHIDE QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jakub, mingo, pfrields
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-10-04 23:53:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 150224    

Description David Woodhouse 2006-10-03 23:18:10 UTC
Section of code from bluez-gnome package:


static int attached_adapters(void)
{
	int count = 0;

	void do_count(gpointer data, gpointer user_data)
	{
		struct adapter_data *adapter = data;

		if (adapter->attached)
			count++;
	}
	g_printerr("Address of do_count is %p\n", do_count);
	g_list_foreach(adapter_list, do_count, NULL);

	return count;
}

Result...

Address of do_count is 0xfc03f2f4

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -134309904 (LWP 31965)]
0xfc03f2f4 in ?? ()
(gdb) bt
#0  0xfc03f2f4 in ?? ()
#1  0x0ee44454 in g_list_foreach () from /lib/libglib-2.0.so.0
#2  0x10004ae8 in attached_adapters () at main.c:647
#3  0x10005538 in main (argc=1, argv=0xfc03f5f4) at main.c:926

It seems that do_count() is on the stack.

Comment 1 David Woodhouse 2006-10-03 23:22:50 UTC
hm, readelf -l shows the stack _is_ marked as executable:
   GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4

Reassigning to kernel.


Comment 2 David Woodhouse 2006-10-03 23:23:51 UTC
I think this is a recent change -- this same binary was working this morning,
before I upgraded the kernel and rebooted.

Comment 3 David Woodhouse 2006-10-03 23:47:53 UTC
Allegedly happens on i386+NX too. We made non-exec-stack the default, and the
code to handle PT_GNU_STACK doesn't seem to be working.

Comment 4 David Woodhouse 2006-10-04 00:10:42 UTC
Should be fixed in 2.6.18-1.2734, building now. The option documented as
'default no-exec-stack' is in fact _force_ no-exec-stack, regardless of the
PT_GNU_STACK of the binary. We'd turned it on.

I've turned it off again and corrected the documentation immediately below it.

Comment 5 David Woodhouse 2006-10-04 07:46:48 UTC
This is a repeat of bug 187853.

Back to the GCC question though -- why couldn't we achieve the above without
having to put code on the stack?

A standalone test case...

typedef int (*fn)(int *);

void invoke_fn(fn foofn, int *x)
{
        foofn(x);
}

int main(void)
{
        int y = 0;

        int do_something(int *foo)
        {
                foo+=y;
        }

        invoke_fn(&do_something, &y);

        printf("%d\n", y);
        return 0;
}

Am I missing some reason (other than the scope and visibilty of 'y' and
'do_something()' which presumably it wouldn't be hard to get right) why the
compiler can't treat that fairly much as it does if you apply the following
patch (i.e. move both 'y' and 'do_something()' out of main()'s scope).:

--- foo.c       2006-10-04 00:45:07.000000000 +0100
+++ foo2.c      2006-10-04 08:44:43.000000000 +0100
@@ -8,14 +8,16 @@ void invoke_fn(fn foofn, int *x)
        foofn(x);
 }
 
-int main(void)
+static int y = 0;
+
+static int do_something(int *foo)
 {
-       int y = 0;
+       foo+=y;
+}
 
-       int do_something(int *foo)
-       {
-               foo+=y;
-       }
+int main(void)
+{
+       y = 0;
 
        invoke_fn(&do_something, &y);
 


Comment 6 Jakub Jelinek 2006-10-04 08:14:27 UTC
How can you turn an automatic variable into a static one?
Even __thread doesn't help you, the function can be called recursively and
you are supposed to have a new instance of the variable in every function
invocation.  Also, the nested function pointer can be passed throguh arbitrarily
many functions in between, accross many translation units, so the function
pointer really has to act as a function pointer.

On architectures that have fat function pointers (ia64, ppc64, hppa) you don't
need executable trampolines, as you can encode the static chain pointer into
the second or third word in the fat pointer.

But on the other arches, all you could do to avoid executable trampolines would
be to have a pool of stubs compiled into the application (but that would in
itself limit the total number of nested function pointers that can be live at
one point) and do some garbage collection on them (which would be certainly
non-trivial).

The toolchain has PT_GNU_STACK for a reason, when the SELinux policy decides
not to honor those and track them in yet another way is not toolchain's fault.

Comment 7 David Woodhouse 2006-10-04 08:30:06 UTC
(In reply to comment #6)
> How can you turn an automatic variable into a static one?

Doh. Good point; thanks for pointing it out.