linux-kernel - Re: [PATCH v5] PM / wakeup: show wakeup sources stats in sysfs

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: <5d40d5b3.1c69fb81.6047f.1cc3@mx.google.com>
Date:   Tue, 30 Jul 2019 16:41:38 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Tri Vo <trong@...roid.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Hridya Valsaraju <hridya@...gle.com>,
        Sandeep Patil <sspatil@...gle.com>,
        Kalesh Singh <kaleshsingh@...gle.com>,
        Ravi Chandra Sadineni <ravisadineni@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        "Cc: Android Kernel" <kernel-team@...roid.com>,
        kbuild test robot <lkp@...el.com>
Subject: Re: [PATCH v5] PM / wakeup: show wakeup sources stats in sysfs

Quoting Rafael J. Wysocki (2019-07-30 16:05:55)
> On Wed, Jul 31, 2019 at 12:26 AM Stephen Boyd <swboyd@...omium.org> wrote:
> >
> > Quoting Rafael J. Wysocki (2019-07-30 15:17:55)
> > > On Tuesday, July 30, 2019 8:48:09 PM CEST Stephen Boyd wrote:
> > > >
> > > > Using the same prefix for the class and the device name is quite common.
> > > > For example, see the input, regulator, tty, tpm, remoteproc, hwmon,
> > > > extcon classes. I'd prefer it was left as /sys/class/wakeup/wakeupN. The
> > > > class name could be changed to wakeup_source perhaps (i.e.
> > > > /sys/class/wakeup_source/wakeupN)?
> > >
> > > Alternatively /sys/class/wakeup/wsN
> > >
> >
> > Or /sys/class/wakeup/eventN? It's your bikeshed to paint.
> 
> So actually the underlying problem here is that device_wakeup_enable()
> tries to register a wakeup source and then attach it to the device to
> avoid calling possibly sleeping functions under a spinlock.

Agreed, that is one problem.

> 
> However, it should be possible to call wakeup_source_create(name)
> first, then attach the wakeup source to the device (after checking for
> presence), and then invoke wakeup_source_add() (after dropping the
> lock).  If the wakeup source virtual device registration is done in
> wakeup_source_add(), that should avoid the problem altogether without
> having to introduce extra complexity.

While reordering the code to do what you describe will fix this specific
duplicate name problem, it won't fix the general problem with reusing
device names from one bus on a different bus/class. We can run into the
same problem when two buses name their devices the same name and then we
attempt to attach a wakeup source to those two devices. Or we can have a
problem where a virtual wakeup is made with the same name, and again
we'll try to make a duplicate named device. Using something like 'event'
or 'wakeup' or 'ws' as the prefix avoids this problem and keeps things
clean.

We should probably avoid letting the same virtual wakeup source be made
with the same name anyway, because userspace will be confused about what
virtual wakeup it is otherwise. I concede that using the name of the
wakeup source catches this problem without adding extra code.

Either way, I'd like to see what you outline implemented so that we
don't need to do more work than is necessary when userspace writes to
the file. I just don't want to see us need to change the name of the
wakeup device later on and then add a 'name' attribute to the class so
that we can avoid name collisions due to various buses controlling the
string we use to create the name of the wakeup device.