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, 29 Apr 2024 08:26:49 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Dan Williams <dan.j.williams@...el.com>, linux-kernel@...r.kernel.org,
 x86@...nel.org, linux-coco@...ts.linux.dev, svsm-devel@...onut-svsm.dev
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Michael Roth <michael.roth@....com>,
 Ashish Kalra <ashish.kalra@....com>, Joel Becker <jlbec@...lplan.org>,
 Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v4 12/15] fs/configfs: Add a callback to determine
 attribute visibility

On 4/26/24 16:48, Dan Williams wrote:
> Tom Lendacky wrote:
>> In order to support dynamic decisions as to whether an attribute should be
>> created, add a callback that returns a bool to indicate whether the
>> attribute should be displayed. If no callback is registered, the attribute
>> is displayed by default.
>>
>> Cc: Joel Becker <jlbec@...lplan.org>
>> Cc: Christoph Hellwig <hch@....de>
>> Co-developed-by: Dan Williams <dan.j.williams@...el.com>
>> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>>   fs/configfs/dir.c        | 20 ++++++++++++++++++++
>>   include/linux/configfs.h |  3 +++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index 18677cd4e62f..463e66258507 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
>>   static int populate_attrs(struct config_item *item)
>>   {
>>   	const struct config_item_type *t = item->ci_type;
>> +	struct configfs_group_operations *ops;
>>   	struct configfs_attribute *attr;
>>   	struct configfs_bin_attribute *bin_attr;
>>   	int error = 0;
>> @@ -587,14 +588,33 @@ static int populate_attrs(struct config_item *item)
>>   
>>   	if (!t)
>>   		return -EINVAL;
>> +
>> +	ops = t->ct_group_ops;
>> +	if (!ops) {
>> +		struct config_group *g = item->ci_group;
>> +
>> +		/*
>> +		 * No item specific group operations, check if the item's group
>> +		 * has group operations.
>> +		 */
>> +		if (g && g->cg_item.ci_type)
>> +			ops = g->cg_item.ci_type->ct_group_ops;
> 
> Oh, I would not have expected to need to consider any alternate group
> ops for attribute visibility beyond t->ct_group_ops. However in my RFC
> example I made this mistake:

Right, that's the question. Do you use the default group ops that are 
created for TSM report or do you require the ops be part of the 
attributes. It is definitely easy to do the latter.

> 
>    static struct configfs_group_operations tsm_report_group_ops = {
>           .make_item = tsm_report_make_item,
> +        .is_visible = tsm_report_attr_visible,
> +        .is_bin_visible = tsm_report_bin_attr_visible,
>    };
>   
> Which in retrospect is the wrong level, and I suspect only reachable if
> you do the the above awkward indirection ("ops =
> g->cg_item.ci_type->ct_group_ops"). Instead, I was expecting symmetry
> with sysfs where the object that carries ->attrs also carries
> ->is_visible, so something like this:
> 
> + static struct configfs_group_operations tsm_report_attr_group_ops = {
> +         .is_visible = tsm_report_attr_visible,
> +         .is_bin_visible = tsm_report_bin_attr_visible,
> + };
> 
>    const struct config_item_type tsm_report_type = {
>            .ct_owner = THIS_MODULE,
>            .ct_bin_attrs = tsm_report_bin_attrs,
>            .ct_attrs = tsm_report_attrs,
>            .ct_item_ops = &tsm_report_item_ops,
> +         .ct_group_ops = &tsm_report_attr_group_ops
>    };
>    EXPORT_SYMBOL_GPL(tsm_report_default_type);
> 
> ..because is_visible() at the g->cg_item.ci_type->ct_group_ops level
> would seem to mean parent directory visibility which is mismatched.
> 
> However as I stare at this a bit more it sinks in that configfs "group"
> != sysfs "group". So I am open to the suggestion that ci_item_ops is the
> right place to house item attribute visibility callbacks, or even a new
> "ci_attr_ops" expressly for this purpose. Either way my expectation is
> that config_item_type can get to the visibilty callbacks for its
> attributes without needing to traverse any other groups or items.

I'll update the patchset to do that.

Thanks,
Tom

> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ