netdev - Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31

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:   Tue, 1 Nov 2022 06:06:13 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Marc Kleine-Budde <mkl@...gutronix.de>, netdev@...r.kernel.org,
        davem@...emloft.net, linux-can@...r.kernel.org,
        kernel@...gutronix.de
Subject: Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31

On Mon, Oct 31, 2022 at 08:27:14PM -0700, Jakub Kicinski wrote:
> On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
> > The first 7 patches are by Stephane Grosjean and Lukas Magel and
> > target the peak_usb driver. Support for flashing a user defined device
> > ID via the ethtool flash interface is added. A read only sysfs
> 
> nit: ethtool eeprom set != ethtool flash
> 
> > attribute for that value is added to distinguish between devices via
> > udev.
> 
> So the user can write an arbitrary u32 value into flash which then
> persistently pops up in sysfs across reboots (as a custom attribute
> called "user_devid")?
> 
> I don't know.. the whole thing strikes me as odd. Greg do you have any
> feelings about such.. solutions?
> 
> patches 5 and 6 here:
> https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/

Device-specific attributes should be in the device-specific directory,
not burried in a class directory somewhere that is generic like this one
is.

Why isn't this an attribute of the usb device instead?

And there's no need to reorder the .h file includes in patch 06 while
you are adding a sysfs entry, that should be a separate commit, right?

Also, the line:

+	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,

Is odd, there should never be a need to cast anything like this if you
are doing things properly.

So this still needs work, sorry.

thanks,

greg k-h