linux-kernel - Re: [PATCH] PM: wakeup: Add a missing return case in init

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0jbHwiZemtNAoM-jmgB_58VqmKUkqv4P7qrPkxWzBzMyQ@mail.gmail.com>
Date: Fri, 15 Mar 2024 13:11:11 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Dhruva Gole <d-gole@...com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Tony Lindgren <tony@...mide.com>, theo.lebrun@...tlin.com, 
	Sibi Sankar <quic_sibis@...cinc.com>, Viresh Kumar <viresh.kumar@...aro.org>, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH] PM: wakeup: Add a missing return case in init_wakeup

On Fri, Mar 15, 2024 at 6:18 AM Dhruva Gole <d-gole@...com> wrote:
>
> On Mar 14, 2024 at 16:29:36 +0100, Rafael J. Wysocki wrote:
> > On Thu, Mar 14, 2024 at 4:18 PM Dhruva Gole <d-gole@...com> wrote:
> > >
> > > Hi,
> > >
> > > On Mar 14, 2024 at 15:01:36 +0100, Rafael J. Wysocki wrote:
> > > > On Thu, Mar 14, 2024 at 8:55 AM Dhruva Gole <d-gole@...com> wrote:
> > > > >
> > > > > The device_wakeup_disable call can return an error if no dev exists
> > > > > however this was being ignored. Catch this return value and propagate it
> > > > > onward in device_init_wakeup.
> > > >
> > > > Why does this matter to the callers of device_init_wakeup()?
> > >
> > > If atall !dev->power.can_wakeup then the caller should know something is
> > > funny right?
> >
> > What would the caller do with this information?
> >
> > They attempted to disable wakeup on a device that doesn't exist or is
> > not wake-capable, and so what?
>
> Using drivers/char/hw_random/xgene-rng.c as an example, we can atleast
> print a warning or something to make the user aware of an unclean state.
>
> Is the argument here that if the caller can't do anything meaningful
> then what's the point of returning any error?
>
> If so, then my preference would be just to propagate as much information
> upward from the stack whether the caller can make use of that error and
> in what way is upto the caller, if nothing else then even a warn or
> error print is still useful piece of information.

I'm not making a general argument, just talking about this particular case.

The only error code returned by device_wakeup_disable() is -EINVAL and
it is returned when nothing had to be done because it was not
applicable to the argument passed by the caller.

Quite frankly, I don't see why any caller of device_init_wakeup()
passing false as its second argument would be interested in handling
that error.

> However if it's useless to return anything from device_wakeup_disable
> then we might as well make that function a void or something and avoid
> any confusion as to if there's any point in returning error at that
> point.

That would work for me.