* [PATCH] firmware: Prevent oops on delayed abort @ 2017-01-04 9:31 Chris Wilson 2017-01-04 11:12 ` Ming Lei 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2017-01-04 9:31 UTC (permalink / raw) To: linux-kernel Cc: Chris Wilson, Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman If the firmware load is slow and cancelled, we may call fw_load_abort() twice and promptly oops on the second with a NULL dereference. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ming Lei <ming.lei@canonical.com> Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/base/firmware_class.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 4497d263209f..d03e21c1d2f3 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -557,6 +557,9 @@ static void fw_load_abort(struct firmware_priv *fw_priv) { struct firmware_buf *buf = fw_priv->buf; + if (!buf) + return; + __fw_load_abort(buf); /* avoid user action after loading abort */ -- 2.11.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] firmware: Prevent oops on delayed abort 2017-01-04 9:31 [PATCH] firmware: Prevent oops on delayed abort Chris Wilson @ 2017-01-04 11:12 ` Ming Lei 2017-01-04 15:40 ` Luis R. Rodriguez 0 siblings, 1 reply; 5+ messages in thread From: Ming Lei @ 2017-01-04 11:12 UTC (permalink / raw) To: Chris Wilson Cc: Linux Kernel Mailing List, Luis R. Rodriguez, Greg Kroah-Hartman On Wed, Jan 4, 2017 at 5:31 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > If the firmware load is slow and cancelled, we may call fw_load_abort() > twice and promptly oops on the second with a NULL dereference. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ming Lei <ming.lei@canonical.com> > Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/base/firmware_class.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > index 4497d263209f..d03e21c1d2f3 100644 > --- a/drivers/base/firmware_class.c > +++ b/drivers/base/firmware_class.c > @@ -557,6 +557,9 @@ static void fw_load_abort(struct firmware_priv *fw_priv) > { > struct firmware_buf *buf = fw_priv->buf; > > + if (!buf) > + return; > + > __fw_load_abort(buf); > > /* avoid user action after loading abort */ > -- > 2.11.0 > Looks fine, Acked-by: Ming Lei <ming.lei@canonical.com> Thanks, Ming ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] firmware: Prevent oops on delayed abort 2017-01-04 11:12 ` Ming Lei @ 2017-01-04 15:40 ` Luis R. Rodriguez 2017-01-04 15:47 ` Chris Wilson 0 siblings, 1 reply; 5+ messages in thread From: Luis R. Rodriguez @ 2017-01-04 15:40 UTC (permalink / raw) To: Ming Lei Cc: Chris Wilson, Linux Kernel Mailing List, Luis R. Rodriguez, Greg Kroah-Hartman On Wed, Jan 04, 2017 at 07:12:11PM +0800, Ming Lei wrote: > On Wed, Jan 4, 2017 at 5:31 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > If the firmware load is slow and cancelled, we may call fw_load_abort() > > twice and promptly oops on the second with a NULL dereference. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ming Lei <ming.lei@canonical.com> > > Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > drivers/base/firmware_class.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > index 4497d263209f..d03e21c1d2f3 100644 > > --- a/drivers/base/firmware_class.c > > +++ b/drivers/base/firmware_class.c > > @@ -557,6 +557,9 @@ static void fw_load_abort(struct firmware_priv *fw_priv) > > { > > struct firmware_buf *buf = fw_priv->buf; > > > > + if (!buf) > > + return; > > + > > __fw_load_abort(buf); > > > > /* avoid user action after loading abort */ > > -- > > 2.11.0 > > > > Looks fine, > > Acked-by: Ming Lei <ming.lei@canonical.com> But does this fix an actual oops or just theoretical? If it does can you include the trace, and amend the commit log to Cc stable so this gets properly propagated into the stable kernels? Luis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] firmware: Prevent oops on delayed abort 2017-01-04 15:40 ` Luis R. Rodriguez @ 2017-01-04 15:47 ` Chris Wilson 2017-01-04 16:50 ` Luis R. Rodriguez 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2017-01-04 15:47 UTC (permalink / raw) To: Luis R. Rodriguez; +Cc: Ming Lei, Linux Kernel Mailing List, Greg Kroah-Hartman On Wed, Jan 04, 2017 at 04:40:49PM +0100, Luis R. Rodriguez wrote: > On Wed, Jan 04, 2017 at 07:12:11PM +0800, Ming Lei wrote: > > On Wed, Jan 4, 2017 at 5:31 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > If the firmware load is slow and cancelled, we may call fw_load_abort() > > > twice and promptly oops on the second with a NULL dereference. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Ming Lei <ming.lei@canonical.com> > > > Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > --- > > > drivers/base/firmware_class.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > > index 4497d263209f..d03e21c1d2f3 100644 > > > --- a/drivers/base/firmware_class.c > > > +++ b/drivers/base/firmware_class.c > > > @@ -557,6 +557,9 @@ static void fw_load_abort(struct firmware_priv *fw_priv) > > > { > > > struct firmware_buf *buf = fw_priv->buf; > > > > > > + if (!buf) > > > + return; > > > + > > > __fw_load_abort(buf); > > > > > > /* avoid user action after loading abort */ > > > -- > > > 2.11.0 > > > > > > > Looks fine, > > > > Acked-by: Ming Lei <ming.lei@canonical.com> > > But does this fix an actual oops or just theoretical? If it does can you > include the trace, and amend the commit log to Cc stable so this gets > properly propagated into the stable kernels? Actual oops, I was going to include the oops from dmesg but that was lost on a the forced reboot. It's fairly clear the cause once you read the callers. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] firmware: Prevent oops on delayed abort 2017-01-04 15:47 ` Chris Wilson @ 2017-01-04 16:50 ` Luis R. Rodriguez 0 siblings, 0 replies; 5+ messages in thread From: Luis R. Rodriguez @ 2017-01-04 16:50 UTC (permalink / raw) To: Chris Wilson Cc: Luis R. Rodriguez, Ming Lei, Linux Kernel Mailing List, Greg Kroah-Hartman On Wed, Jan 04, 2017 at 03:47:49PM +0000, Chris Wilson wrote: > On Wed, Jan 04, 2017 at 04:40:49PM +0100, Luis R. Rodriguez wrote: > > On Wed, Jan 04, 2017 at 07:12:11PM +0800, Ming Lei wrote: > > > On Wed, Jan 4, 2017 at 5:31 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > If the firmware load is slow and cancelled, we may call fw_load_abort() > > > > twice and promptly oops on the second with a NULL dereference. > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > Cc: Ming Lei <ming.lei@canonical.com> > > > > Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > --- > > > > drivers/base/firmware_class.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > > > index 4497d263209f..d03e21c1d2f3 100644 > > > > --- a/drivers/base/firmware_class.c > > > > +++ b/drivers/base/firmware_class.c > > > > @@ -557,6 +557,9 @@ static void fw_load_abort(struct firmware_priv *fw_priv) > > > > { > > > > struct firmware_buf *buf = fw_priv->buf; > > > > > > > > + if (!buf) > > > > + return; > > > > + > > > > __fw_load_abort(buf); > > > > > > > > /* avoid user action after loading abort */ > > > > -- > > > > 2.11.0 > > > > > > > > > > Looks fine, > > > > > > Acked-by: Ming Lei <ming.lei@canonical.com> > > > > But does this fix an actual oops or just theoretical? If it does can you > > include the trace, and amend the commit log to Cc stable so this gets > > properly propagated into the stable kernels? > > Actual oops, I was going to include the oops from dmesg but that was > lost on a the forced reboot. It's fairly clear the cause once you read > the callers. I see, if you can reproduce and include it that would be greatly appreciated, and please re-submit and Cc stable on the commit log, if you can indicate as of what kernel this is needed even better (Fixes: tag might help). Luis ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-04 17:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-04 9:31 [PATCH] firmware: Prevent oops on delayed abort Chris Wilson 2017-01-04 11:12 ` Ming Lei 2017-01-04 15:40 ` Luis R. Rodriguez 2017-01-04 15:47 ` Chris Wilson 2017-01-04 16:50 ` Luis R. Rodriguez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®