linux-kernel - Re: [PATCH 1/3] firmware: Avoid superfluous usermodehelper lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACVXFVPo7ED5fo1+FT=ibM_HvB2h1Vc7P2e+C_LHGw-BQ9e-tA@mail.gmail.com>
Date:	Sat, 11 May 2013 21:01:27 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 1/3] firmware: Avoid superfluous usermodehelper lock

On Fri, May 10, 2013 at 5:32 PM, Takashi Iwai <tiwai@...e.de> wrote:
> At Fri, 10 May 2013 09:25:51 +0800,
>>
>> Anyway, if you want to force killing loader, please only kill these
>> FW_ACTION_NOHOTPLUG before suspend and reboot, and do
>> not touch FW_ACTION_HOTPLUG. Is it OK for you?
>
> Note that, as with my patch, only the shutdown case is handled.  Let's
> not mixing up suspend and shutdown behavior for now.
>
> I see no reason why we need to wait at shutdown even for
> FW_ACTION_HOTPLUG.  At that point, there should be no longer
> user-space action.  It means that the driver shouldn't get any more
> data to finish the f/w loading upon that point.  Thus the only

For example, when one ethernet driver is requesting its firmware,
the loader is forced to be killed before shutdown, so the driver can't set the
WoL any more in its shutdown(), then users start to complain it is a
regression.

That is why I suggest you to only kill requests of FW_ACTION_NOHOTPLUG.

Also it isn't good to affect all drivers just for fixing two insane drivers.

> possible consequence is the timeout, which is equivalent with the
> immediate abort of the operation.
>
> As mentioned earlier, the suspend behavior may be different.  We want
> to retry the f/w load.  Ideally, the f/w loader should abort and
> automatically retry after resume.  In this case, also there is no big

I don't think it is good idea since suspend() may need firmware to
change the device's power state. Considered that only two insane
drivers use FW_ACTION_NOHOTPLUG, we can kill the two before
suspend just like the case of shutdown.

> reason to distinguish FW_ACTION_* types.  Even for udev case, the
> action can be easily retried.
>
> Or, a cleaner solution would be to get rid of FW_ACTION_NOHOTPLUG
> completely.  As Kay mentioned, this was a big mistake from the very

It is still not a good idea, hackers may need FW_ACTION_NOHOTPLUG
to debug its driver with private firmwares, or examples like dell's BIOS update.


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