Bug 9578 - Returning from a function with a pointer to a local variable on kernel/kmod.c
Summary: Returning from a function with a pointer to a local variable on kernel/kmod.c
Status: REJECTED INVALID
Alias: None
Product: Other
Classification: Unclassified
Component: Modules (show other bugs)
Hardware: All Linux
: P1 low
Assignee: other_modules
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-15 14:12 UTC by Marcio Buss
Modified: 2007-12-16 13:07 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6.23
Subsystem:
Regression: ---
Bisected commit-id:


Attachments

Description Marcio Buss 2007-12-15 14:12:07 UTC
This is a low severity bug, but I thought it was worth reporting it. 
There is a valid execution path within procedure "call_usermodehelper_exec"
which returns to the caller with "sub_info->complete" pointing-to
a local variable ("done"). The error can be tracked down as:

(1) the statement at line 467 executes establishing the points-to relation
(2) the test at line 471 is true, making the procedure to return.

sub_info->complete now points-to "done," which has been deallocated.
Comment 1 Oleg Nesterov 2007-12-16 10:07:45 UTC
> This is a low severity bug,

Well, I am not sure this is bug.

> (2) the test at line 471 is true, making the procedure to return.
>
> sub_info->complete now points-to "done," which has been deallocated.

We return when wait == UMH_NO_WAIT, but UMH_NO_WAIT means "don't wait"
and thus "don't use info->complete".

Please close this bug unless you see other problems.
Comment 2 Marcio Buss 2007-12-16 13:07:30 UTC
Sure, I will close the bug.

PS1.: You are right, it is not a bug---but referring
to a deallocated block of memory makes "bugs-to-be"
happy :) 
Comment 3 Oleg Nesterov 2007-12-16 14:16:45 UTC
On 12/16, bugme-daemon@bugzilla.kernel.org wrote:
> 
> but referring
> to a deallocated block of memory makes "bugs-to-be"
> happy :) 

Yes, you are right. And this "/* task has freed sub_info */" comment
is obviously wrong.

We can do something like

	if (wait != UMH_NO_WAIT)
		sub_info->complete = &done;
	sub_info->wait = wait;
	...

but then another automatic tool can report the "possible usage of
uninitialized subprocess_info->complete". And the code above imho
is more unclear compared to what we currently have.

Just in case, coredump_wait() sets ->core_startup_done = startup_done
and returns if zap_threads() fails. Not a bug ;)

In any case, thanks for these reports. The extra review is always better
than a missed bug.

Oleg.

Note You need to log in before you can comment on or make changes to this bug.