Re: [PATCH v2 1/5] firmware: fix __getname() missing failure check

From: Greg KH
Date: Tue May 12 2015 - 17:24:38 EST


On Tue, May 12, 2015 at 11:30:53AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
>
> The request_firmware*() APIs uses __getname() to iterate
> over the list of paths possible for firmware to be found,
> the code however never checked for failure on __getname().
> Although *very unlikely*, this can still happen. Add the
> missing check.
>
> There is still no checks on the concatenation of the path
> and filename passed, that requires a bit more work and
> subsequent patches address this. The commit that introduced
> this is abb139e7 ("firmware: teach the kernel to load
> firmware files directly from the filesystem").
>
> mcgrof@ergon ~/linux (git::firmware-fixes) $ git describe --contains abb139e7
> v3.7-rc1~120
>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxxxxx>
> Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> Cc: David Howells <dhowells@xxxxxxxxxx>
> Cc: Kyle McMartin <kyle@xxxxxxxxxx>
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxx>
> ---
> drivers/base/firmware_class.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 171841a..bc6c8e6 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -322,7 +322,11 @@ static int fw_get_filesystem_firmware(struct device *device,
> {
> int i;
> int rc = -ENOENT;
> - char *path = __getname();
> + char *path;
> +
> + path = __getname();
> + if (unlikely(!path))

Please only use likely/unlikely on code paths that actually care about
it (i.e. you can measure the difference). Otherwise it is pretty
useless, and people have determined that sometimes it is slower as
humans get this wrong a lot of time...

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/