From: Greg KH <gregkh@linuxfoundation.org>
To: Shuah Khan <shuah.kh@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
len.brown@intel.com, pavel@ucw.cz, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, shuahkhan@gmail.com,
anton@enomsg.org, dwmw2@infradead.org, stable@vger.kernel.org
Subject: Re: [PATCH] power: Change device_wakeup_enable() to check for null dev_name(dev)
Date: Fri, 15 Nov 2013 16:25:01 -0800 [thread overview]
Message-ID: <20131116002501.GA20625@kroah.com> (raw)
In-Reply-To: <5286B95F.4040306@samsung.com>
On Fri, Nov 15, 2013 at 05:16:31PM -0700, Shuah Khan wrote:
> On 11/15/2013 05:21 PM, Rafael J. Wysocki wrote:
> > On Friday, November 15, 2013 05:03:57 PM Shuah Khan wrote:
> >> device_wakeup_enable() uses dev_name(dev) as the wakeup source name.
> >> When it gets called with a device with its name not yet set, ws structure
> >> with ws->name = NULL gets created.
> >>
> >> When kernel is booted with wakeup_source_activate enabled, it will panic
> >> when the trace point code tries to derefernces ws->name.
> >>
> >> Change device_wakeup_enable() to check for dev_name(dev) null condition
> >> and return -EINVAL to avoid panics when device_wakeup_enable() gets called
> >> before device is fully initialized with its name.
> return -EINVAL;
> >
> > Can you please use WARN_ON(!dev_name(dev)) here? While I agree that it is a
> > bad idea to crash the kernel because dev has no name, that indicates a driver
> > bug that shouldn't be too easy to ignore.
> >
> > Thanks!
> >
>
> Right. ok I will re-cut the patch with WARN_ON and send it. fyi I did
> send fix for the driver (power_supply) as well.
>
> http://www.kernelhub.org/?msg=362354&p=2
Why is a driver calling kobject_set_name() instead of device_set_name()?
Yes, it's really the same thing deep down, but drivers should never care
about a kobject, just 'struct device'. Well, even then it usually
should care about it's type of 'struct device' but that's a different
issue...
Anyway, not saying your patch is wrong at all, just for the future if
people are looking for code cleanups...
thanks,
greg k-h
next prev parent reply other threads:[~2013-11-16 0:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-16 0:03 [PATCH] power: Change device_wakeup_enable() to check for null dev_name(dev) Shuah Khan
2013-11-16 0:21 ` Rafael J. Wysocki
2013-11-16 0:16 ` Shuah Khan
2013-11-16 0:25 ` Greg KH [this message]
2013-11-16 15:28 ` Shuah Khan
2013-11-16 0:32 ` Rafael J. Wysocki
2013-11-18 17:45 ` Shuah Khan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131116002501.GA20625@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=anton@enomsg.org \
--cc=dwmw2@infradead.org \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
--cc=shuah.kh@samsung.com \
--cc=shuahkhan@gmail.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.