Re: [PATCH 1/1] firmware_loading_store: fix firmware loadinguse-after-free oops

[Greg KH]

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Tue, Dec 13, 2011 at 01:31:32PM +0800, DONG-DONG YANG wrote:
> If we do the check for fw in the fw_load_abort call, I concern whether it would
> limit FW_STATUS_ABORT scope, which would presume any fw_abort occur after fw
> instance generation.

Are you sure this really fixes anything?  What happens if the fw pointer
becomes null after the check happens but before the call?  What is
protecting that from happening here?

> The issue was introduced when the webtop components are integrated into the
> Linux Android platform.
> The webtop firmware loading performs udev configuration files (under /lib/udev/
> *) by default, however,
> ueventd is in charge of such loading task on android framework. The conflict
> causes the weird things happen.
>
> 50-firmware.rules, one of udev default rules, forks a sub-process, named
> "firmware", which sends "-1" to "loading" fw ctrl file on receiving "add"
> event. Such firmware loading failure should bring on APP processes exit or
> error report, but it causes use-after-free oops due to we have no check for fw
> in firmware_loading_store.

Sounds like this is a major userspace configuration error.  Not that
this means we shouldn't be applying the kernel patch, just that you need
to fix up your system first :)

Hm, so, you have two different processes trying to write to the firmware
file at the same time?  And when one finishes, the other one tries to
abort things?

Ok, I can understand that, it's a very messed up userspace, but I
understand.

But, your patch needs to properly hold the lock when this is being
checked, otherwise it will still race and could still oops.

And, your check needs to be done in a different place, again, what's to
keep that pointer from going away later, in the middle of that call you
just made?

Care to fix this up properly?

> The patch and log analysis are attached.

Please don't attach patches, it's not easy to apply them when they are
in base64 mode.

Also, your patch can't be applied as you made it against a very wierd
patch "rbase"?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/