Received-SPF: pass (google.com: domain of linux-kernel+bounces-161195-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sat, 27 Apr 2024 14:33:24 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: Lukas Wunner <lukas@wunner.de>, Dan Williams <dan.j.williams@intel.com>
CC: <gregkh@linuxfoundation.org>, Pierre-Louis Bossart
	<pierre-louis.bossart@linux.intel.com>, Marc Herbert
	<marc.herbert@intel.com>, "Rafael J. Wysocki" <rafael@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	<linux-coco@lists.linux.dev>, <alsa-devel@alsa-project.org>
Subject: Re: [PATCH 1/3] sysfs: Fix crash on empty group attributes array
Message-ID: <662d6f24528d7_b6e0294d2@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <170863444851.1479840.10249410842428140526.stgit@dwillia2-xfh.jf.intel.com>
 <170863445442.1479840.1818801787239831650.stgit@dwillia2-xfh.jf.intel.com>
 <ZiYrzzk9Me1aksmE@wunner.de>
 <662beb6ad280f_db82d29458@dwillia2-xfh.jf.intel.com.notmuch>
 <Ziv9984CJeQ4muZy@wunner.de>
 <662d2ca522cc6_b6e02942d@dwillia2-mobl3.amr.corp.intel.com.notmuch>
 <Zi1qtKNwcyydP4c2@wunner.de>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Zi1qtKNwcyydP4c2@wunner.de>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?9ZqRktJm2AY5zuPwhGB8rA9Sm6VxKUjtJrTWLZPOp23TqP1497d4Zog3cThc?=
 =?us-ascii?Q?VJLWuQCcJo70P7mlYPj2iWl5CoxEpgNoYDjXlpY8JMcPj4EBVtNlpAe1Anp7?=
 =?us-ascii?Q?/2F0hWmpHkpDgVgkb1itTZ004k5HGoaOd2wMmBR2xIB7e17J0KX28ttgv1Nq?=
 =?us-ascii?Q?k6O1XnVkRI+EGTtSSN4xzg+RlpGFaj94oVUEuX9N+UMy+vKuzOqPzsAjpu1V?=
 =?us-ascii?Q?dz3C80vkpXeXx469/b/NjJyrP16fhoTYmPKcCz99EZzppIDI8JymIWMALqTi?=
 =?us-ascii?Q?sx0n71+lV1DO9mrsvoU5BX1AfiQsuxKT7UTAiSCVz7Z7ghj9QD0JJwN3KADr?=
 =?us-ascii?Q?IvUwgH/8+lee9Vv3H9H59CyVRHGE5GUM8pqMDPooRWJwyR8WgTXkpj3amFT5?=
 =?us-ascii?Q?URIwFdloOD7Bfb6KIWPCd53bfhRDSl/NoILpwhQPgI7jo3IBPUhNuMObsLMg?=
 =?us-ascii?Q?YIZpCh3xH6gRbE9Bt5XCeZdcyAkuvVojvhNCIiFFZu4AF44VS4Pm1BSidssB?=
 =?us-ascii?Q?wlnGgKrvZNMZcppSWtgLLP7SVTSD8mWWfUuvuFWFSotY5eVYBvczIdfQViop?=
 =?us-ascii?Q?/XTUZc4u++0TExmdYZBL9xgnlFkZu06NBhytyQlYmkcmTa0ttst7aXuw8wkO?=
 =?us-ascii?Q?kpa8PdaPtE9iahs6gO6momZbGCZwQBoC101z6Qvoms26F1yv+xX1RDrYtO6r?=
 =?us-ascii?Q?qC6m24EXhdGChlNhkrKnWq142kyXXh2ZnUJmjj0NXDERkHs/BTfPXQvlILYh?=
 =?us-ascii?Q?YmE5eEc+7r8voywyEDmYPXqRlXrKqj+IhX9g0UyXBnTdsvHfj+sOC4wCu4M6?=
 =?us-ascii?Q?XBDDZYfnB3FZRs6Uyh1F1lxSrwzQZjnH0AcUNDKx0pf9i9xY85t/4iuRjR4o?=
 =?us-ascii?Q?adJ7FLZP99l/PHz28acv+A5ZIt0ZkWC0uSVg7VqlZcMUxP3nPDtBcT++MJhB?=
 =?us-ascii?Q?n4FvqkNvTtSEi0eJIUFJXqXx9UZeW4P06z8RDbhx10uY0HJLo0xcFjo4Ujgp?=
 =?us-ascii?Q?3EcoBBAVkKUW+weI/d+GVlydHuG2cOTfCW9b9qCCmTwGGXaTKrOAPe+RG4Ey?=
 =?us-ascii?Q?oO206Ko/gd72K7JmkT9AyW/scm19azJ9+id8nPY1D3cTgqlG+O0eyjUmzkQY?=
 =?us-ascii?Q?uPrOUjvuyqG0lBq7KsCWByde7atcLggirJsfbPunhU9B9xgmASYxEy7AgMK/?=
 =?us-ascii?Q?tvnafIn7aDw02Fop/C6QyJ5Kzr4ITxthfGtnTk4RqziINr8cWJLAys9Iv7Hk?=
 =?us-ascii?Q?myWLEiu6qTfegsrrnn6I3jis34f6KqAm0GlZOJ8ZSZI/14Xg/r7GD2TpWLnq?=
 =?us-ascii?Q?d9mqSLsBqRTgyhF1q8+ZcMfrQO/0/tuPXl9sJLWdO1hlOIKoOHWkTr4XY403?=
 =?us-ascii?Q?lQaHPKa+DbxvUV9kQob4+QZNLlXxdUgYbTWYBnwNN3RcTyNm0+GJtjKS01Nj?=
 =?us-ascii?Q?ZtVi59tLXre6PkquarApgT+LXe5RvwQ9fzK9WZ6b3rkdptUSBy1KbMEh+sn+?=
 =?us-ascii?Q?45bJ8yEoKZklFI5Dl+UKnuDNHkbGR0gLbgTSgAkvjh7ErY/x4zuNaX4PSaAe?=
 =?us-ascii?Q?2U6f8e0Jsz2nlVFkxawLx5XW3xX6FeqgPnG5airJmOLFFMAsL87QXuSQl32M?=
 =?us-ascii?Q?uw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 104f8490-26ce-4d7b-ea4d-08dc6701ac85
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Apr 2024 21:33:26.8346
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: BOKexdtfjHwmnLSbYOJW9TJbXVqOcqbYdM5Mb+nf7t0ZjJi1xeWaI5dnJcvS8bxIEhCenBw0bUqcZQFo1ij/REK9yZXFYfOamwaxoYIod68=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5044
X-OriginatorOrg: intel.com

Lukas Wunner wrote:
> On Sat, Apr 27, 2024 at 09:49:41AM -0700, Dan Williams wrote:
> > Lukas Wunner wrote:
> > > But I want to raise awareness that the inability to hide
> > > empty attribute groups feels awkward.
> > 
> > That is fair, it was definitely some gymnastics to only change user
> > visible behavior for new "invisible aware" attribute groups that opt-in
> > while leaving all the legacy cases alone.
> > 
> > The concern is knowing when it is ok to call an is_visible() callback
> > with a NULL @attr argument, or knowing when an empty array actually
> > means "hide the group directory".
> > 
> > We could add a sentinel value to indicate "I am an empty attribute list
> > *AND* I want my directory hidden by default". However, that's almost
> > identical to requiring a placeholder attribute in the list just to make
> > __first_visible() happy.
> > 
> > Other ideas?
> 
> Perhaps an optional ->is_group_visible() callback in struct attribute_group
> which gets passed only the struct kobject pointer?
> 
> At least for PCI device authentication, that would be sufficient.
> I could get from the kobject to the corresponding struct device,
> then determine whether the device supports authentication or not.
> 
> Because it's a new, optional callback, there should be no compatibility
> issues.  The SYSFS_GROUP_INVISIBLE return code from the ->is_visible()
> call for individual attributes would not be needed then, at least in my
> use case.

That's where I started with this, but decided it was overkill to
increase the size of that data structure globally for a small number of
use cases.