linux-kernel - Re: [PATCH RFC] sysfs.txt: add note on available attribute macros

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] [day] [month] [year] [list]

Message-ID: <20190214124412.GC17978@kroah.com>
Date:   Thu, 14 Feb 2019 13:44:12 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Nicholas Mc Guire <hofrat@...dl.org>
Cc:     Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] sysfs.txt: add note on available attribute macros

On Thu, Feb 14, 2019 at 12:21:50PM +0100, Nicholas Mc Guire wrote:
> The common cases of attributes should probably be using the __ATTR_XXX
> macros to make code more concise and readable but the current sysfs.txt
> does not point developers to those convenience macros. Further there is
> no note in sysfs.txt currently explaining why trying to set a sysfs file
> to mode 0666 will fail.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
> ---
> 
> Note quite sure if this is the right place to place the note on mode 0666 
> but it should be somewhere as any attempt to set 0666 will be reverted 
> to 0664.
> 
>  Documentation/filesystems/sysfs.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
> index 41411b0..c246142 100644
> --- a/Documentation/filesystems/sysfs.txt
> +++ b/Documentation/filesystems/sysfs.txt
> @@ -116,6 +116,22 @@ static struct device_attribute dev_attr_foo = {
>  	.store = store_foo,
>  };
>  
> +Note as stated in include/linux/kernel.h "OTHER_WRITABLE?  Generally
> +considered a bad idea." so trying to set a sysfs file writable for
> +everyone will fail reverting to mode 0664.
> +
> +For this common cases sysfs.h provides convenience macros to
> +make defining attributes easier as well as making code more
> +concise and readable. The above case could be shortened to:
> +
> +static struct device_attribute dev_attr_foo = __ATTR_RW(foo);
> +
> +the list of helpers available is:
> +__ATTR_RO(name): same as above with mode 0444
> +__ATTR_WO(name): same as above with mode 0200
> +__ATTR_RO_MODE(name, mode): allowing to pass in a specific mode
> +__ATTR_RW(name): setting mode to 0644
> +__ATTR_NULL: which sets the name to NULL

__ATTR_NULL is used to end a list of attributes, no one should actually
try to use it :)

And really, all the current users of it should be converted to use the
groups api instead, as it should not be needed.

__ATTR_RO_MODE() needs a bit more description as well.

So the idea of documenting these is great, just needs a bit of tweaking.

thanks,

greg k-h