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
| ||
|
Date: Wed, 30 Jan 2013 13:04:57 +0100 From: Takashi Iwai <tiwai@...e.de> To: Ming Lei <ming.lei@...onical.com> Cc: linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/4] firmware: Refactoring for splitting user-mode helper code At Wed, 30 Jan 2013 19:48:15 +0800, Ming Lei wrote: > > On Wed, Jan 30, 2013 at 7:08 PM, Takashi Iwai <tiwai@...e.de> wrote: > > At Wed, 30 Jan 2013 11:53:14 +0100, > > Takashi Iwai wrote: > >> > >> At Wed, 30 Jan 2013 18:50:05 +0800, > >> Ming Lei wrote: > >> > > >> > On Wed, Jan 30, 2013 at 6:31 PM, Takashi Iwai <tiwai@...e.de> wrote: > >> > > > >> > > But it's supposed to be cached, no? > >> > > >> > Generally it will be cached, but some crazy devices might come as new > >> > device during resume, so we still need to handle the situation. > >> > >> In that case, shouldn't we simply return an error instead of > >> usermodehelper lock (at least for direct loading)? > > > > The patch below is what I have in my mind... > > > > > > Takashi > > > > --- > > From: Takashi Iwai <tiwai@...e.de> > > Subject: [PATCH] firmware: Skip usermodehelper_lock for direct loading > > > > We don't need a user mode helper lock for the direct fw loading. > > This also reduces the use of timeout when user mode helper is > > disabled, thus we can move the code into ifdef block, too. > > > > For avoiding the possible call of request_firmware() without caching, > > add a check of suspend state for the direct loading case, and returns > > immediately if it's called during suspend/resume. Then it proceeds to > > the user mode helper if enabled, or returns the error. > > > > Signed-off-by: Takashi Iwai <tiwai@...e.de> > > --- > > drivers/base/firmware_class.c | 97 ++++++++++++++++++++++++------------------- > > 1 file changed, 54 insertions(+), 43 deletions(-) > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > index f1caad7..c973bb0 100644 > > --- a/drivers/base/firmware_class.c > > +++ b/drivers/base/firmware_class.c > > @@ -88,13 +88,6 @@ enum { > > FW_STATUS_ABORT, > > }; > > > > -static int loading_timeout = 60; /* In seconds */ > > - > > -static inline long firmware_loading_timeout(void) > > -{ > > - return loading_timeout > 0 ? loading_timeout * HZ : MAX_SCHEDULE_TIMEOUT; > > -} > > - > > struct firmware_cache { > > /* firmware_buf instance will be added into the below list */ > > spinlock_t lock; > > @@ -315,6 +308,9 @@ static bool fw_get_filesystem_firmware(struct device *device, > > bool success = false; > > char *path = __getname(); > > > > + if (device->power.is_suspended) > > + return false; > > The device which is requesting firmware is resumed does not mean the > storage device has been resumed, so this check isn't enough. Well, this practically disables the call of request_firmware() in the resume callback without caching. Takashi -- 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/
Powered by blists - more mailing lists