netdev - Re: [PATCH v2 net-next 1/3] led: trig: netdev: Fix requesting offload device

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]

Date: Mon, 7 Aug 2023 23:49:33 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev <netdev@...r.kernel.org>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <rmk+kernel@...linux.org.uk>,
	Simon Horman <simon.horman@...igine.com>,
	Christian Marangi <ansuelsmth@...il.com>
Subject: Re: [PATCH v2 net-next 1/3] led: trig: netdev: Fix requesting
 offload device

Hi Andrew,

On Tue, Aug 08, 2023 at 12:27:10AM +0200, Andrew Lunn wrote:
> > > +	if (supports_hw_control(led_cdev)) {
> > >  		dev = led_cdev->hw_control_get_device(led_cdev);
> > >  		if (dev) {
> > >  			const char *name = dev_name(dev);
> > >  
> > >  			set_device_name(trigger_data, name, strlen(name));
> > >  			trigger_data->hw_control = true;
> > > -			trigger_data->mode = mode;
> > > +
> > > +			rc = led_cdev->hw_control_get(led_cdev, &mode);
> > 
> > Shouldn't there also be something like
> > led_cdev->hw_control_get(led_cdev, 0);
> > in netdev_trig_deactivate then?
> > Because somehow we need to tell the hardware to no longer perform an
> > offloading operation.
> 
> Hi Daniel
> 
> Back from vacation, so getting around to this now.
> 
> Interesting question. I would actually expect the trigger that takes
> its place will set the brightness to what it wants it to default
> it. It is documented that setting the brightness disables any offload.

So setting the brigthness should result in the trigger to be cleared
back to 'none' then, and that would result in calling
netdev_trig_deactivate if it was previously active.

Because otherwise, even if I take care of truning off all hardware
triggers in the led_set_brightness call, the netdev trigger would
still be selected.

> 
> Have you seen a real problem with changing triggers?

Yes, when manually switching from the netdev trigger to none (or
any other trigger), hardware offloading would remain active with
my implementation of the PHY LED driver[1] (which doesn't clear any
offloading related things but only sets/clears a FORCE_ON bit in its
led_set_brightness function).

[1]: https://github.com/dangowrt/linux/commit/439d52d7b80c97ff0c682ec68a70812030c3d79e


Cheers


Daniel