Re: [PATCH v2] firmware: fix async, manual firmware loading

[Luis R. Rodriguez]

On Fri, Nov 11, 2016 at 04:32:17PM +0100, Yves-Alexis Perez wrote:
> When wait_for_completion_interruptible_timeout() is called from
> _request_firmware_load() with a large timeout value (here, MAX_JIFFY_OFFSET
> because it's a an explicit call to the user helper), its return value (a
> long) will overflow when silently casted to int, be stored as a negative
> integer and then treated as an error.
> 
> This bug was introduced in commit 68ff2a00dbf5 ("firmware_loader: handle
> timeout via wait_for_completion_interruptible_timeout()") when a delay work
> was replaced by the call to wait_for_completion_interruptible_timeout().
> 
> Fix this by re-using the timeout variable and only set retval in specific
> cases.
> 
> Signed-off-by: Yves-Alexis Perez <corsac@xxxxxxxxxx>
> Fixes: 68ff2a00dbf5 "firmware_loader: handle timeout via wait_for_completion_interruptible_timeout()"
> Cc: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxxxxx>
> Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Acked-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

Sorry this commit log still does not help kernel maintainers enough, we want
them well informed immediately what the default behaviour is, what the impact
of the issue is, how broad of an issue it is, how this issue is triggered and
exactly how this can happen. I'll massage this a bit myself and re-submit,
as otherwise this work will be done by each and every single Linux kernel
maintainer working on a Linux distribution trying to determine if its correct
to merge this into their tree or not.

I'll re-submit shortly.

  Luis