linux-kernel - Re: [PATCH v6 2/3] sysfs: Add a attr_is_visible function to attribute

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: Fri, 26 Jan 2024 11:07:13 -0800
From: Greg KH <gregkh@...uxfoundation.org>
To: Dan Williams <dan.j.williams@...el.com>
Cc: Alistair Francis <alistair23@...il.com>, bhelgaas@...gle.com,
	linux-pci@...r.kernel.org, Jonathan.Cameron@...wei.com,
	lukas@...ner.de, alex.williamson@...hat.com,
	christian.koenig@....com, kch@...dia.com, logang@...tatee.com,
	linux-kernel@...r.kernel.org, chaitanyak@...dia.com,
	rdunlap@...radead.org, Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v6 2/3] sysfs: Add a attr_is_visible function to
 attribute_group

On Fri, Jan 26, 2024 at 10:58:07AM -0800, Dan Williams wrote:
> Dan Williams wrote:
> > Greg KH wrote:
> > [..]
> > > > 
> > > > Hey Greg,
> > > > 
> > > > I wanted to follow up on this and see if you are able to provide more
> > > > details for reproducing or if you are able to look into it?
> > > 
> > > Last I tried this, it still crashed and would not boot either on my
> > > laptop or my workstation.  I don't know how it is working properly for
> > > you, what systems have you tried it on?
> > > 
> > > I'm not going to be able to look at this for many weeks due to
> > > conference stuff, so if you want to take the series and test it and
> > > hopefully catch my error, that would be great, I'd love to move forward
> > > and get this merged someday.
> > 
> > I mentioned to Lukas that I was working on a "sysfs group visibility"
> > patch and he pointed me to this thread. I will note that I tried to make
> > the "hide group if all attributes are invisible" approach work, but
> > reverted to a "new is_group_visible() callback" approach. I did read
> > through the thread and try to improve the argument in the changelog
> > accordingly.
> > 
> > I do admit to liking the cleanliness (not touching 'struct
> > attribute_group') of the "hide if no visible attribute" approch, but see
> > the criticism of that alternative below, and let me know if it is
> > convincing. I tested it locally with the following hack to make the
> > group disappear every other sysfs_update_group() event:
> 
> Hey Greg,
> 
> Ignore this version:
> 
> ---
> From: Dan Williams <dan.j.williams@...el.com>
> Date: Tue, 23 Jan 2024 20:20:39 -0800
> Subject: [PATCH] sysfs: Introduce is_group_visible() for attribute_groups
> ---
> 
> I am going back to your approach without a new callback, and some fixups
> to avoid unintended directory removal. I will post that shortly with its
> consumer.

Ignore it?  I was just about to write an email that said "maybe this is
the right way forward" :)

What happened to cause it to not be ok?  And if you can find the bug in
the posted patch here, that would be great as well.

thanks,

greg k-h