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 Apr 2024 10:59:06 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Lukas Wunner <lukas@...ner.de>, Dan Williams <dan.j.williams@...el.com>
CC: <gregkh@...uxfoundation.org>, Pierre-Louis Bossart
	<pierre-louis.bossart@...ux.intel.com>, Marc Herbert
	<marc.herbert@...el.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
	<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
	<linux-coco@...ts.linux.dev>, <alsa-devel@...a-project.org>
Subject: Re: [PATCH 1/3] sysfs: Fix crash on empty group attributes array

Lukas Wunner wrote:
[..]
> > --- a/fs/sysfs/group.c
> > +++ b/fs/sysfs/group.c
> > @@ -33,10 +33,10 @@ static void remove_files(struct kernfs_node *parent,
> >  
> >  static umode_t __first_visible(const struct attribute_group *grp, struct kobject *kobj)
> >  {
> > -	if (grp->attrs && grp->is_visible)
> > +	if (grp->attrs && grp->attrs[0] && grp->is_visible)
> >  		return grp->is_visible(kobj, grp->attrs[0], 0);
> >  
> > -	if (grp->bin_attrs && grp->is_bin_visible)
> > +	if (grp->bin_attrs && grp->bin_attrs[0] && grp->is_bin_visible)
> >  		return grp->is_bin_visible(kobj, grp->bin_attrs[0], 0);
> >  
> >  	return 0;
> 
> I'm wondering why 0 is returned by default and not SYSFS_GROUP_INVISIBLE.
> 
> An empty attribute list (containing just the NULL sentinel) will now
> result in the attribute group being visible as an empty directory.
> 
> I thought the whole point was to hide such empty directories.
> 
> Was it a conscious decision to return 0?

Perhaps there should be a comment here because yes, this is on purpose.

> Did you expect breakage if SYSFS_GROUP_INVISIBLE is returned?

Yes, the history is here:

    https://lore.kernel.org/all/YwZCPdPl2T+ndzjU@kroah.com/

..where an initial attempt to hide empty group directories resulted in
boot failures. The concern is that there might be user tooling that
depends on that empty directory. So the SYSFS_GROUP_INVISIBLE behavior
can only be enabled by explicit result from an is_visible() handler.

That way there is no regression potential for legacy cases where the
empty directory might matter.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ