Re: [PATCH v2] powerpc/papr_scm: Limit the readability of 'perf_stats' sysfs attribute

[Ira Weiny <ira.weiny@intel.com>]

On Mon, Sep 07, 2020 at 04:35:40PM +0530, Vaibhav Jain wrote:
> The newly introduced 'perf_stats' attribute uses the default access
> mode of 0444 letting non-root users access performance stats of an
> nvdimm and potentially force the kernel into issuing large number of
> expensive HCALLs. Since the information exposed by this attribute
> cannot be cached hence its better to ward of access to this attribute
	                                   ^^^
                                           off?

> from users who don't need to access these performance statistics.
> 
> Hence this patch updates access mode of 'perf_stats' attribute to
> be only readable by root users.

Generally it is bad form to say "this patch".  See 4c here:

	-- https://www.ozlabs.org/~akpm/stuff/tpp.txt

But I'm not picky...  :-D

With the s/of/off/ change:

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> Fixes: 2d02bf835e573 ('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Change-log:
> 
> v2:
> * Instead of checking for perfmon_capable() inside show_perf_stats()
>   set the attribute as DEVICE_ATTR_ADMIN_RO [ Aneesh ]
> * Update patch description
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index f439f0dfea7d1..a88a707a608aa 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -822,7 +822,7 @@ static ssize_t perf_stats_show(struct device *dev,
>  	kfree(stats);
>  	return rc ? rc : seq_buf_used(&s);
>  }
> -DEVICE_ATTR_RO(perf_stats);
> +DEVICE_ATTR_ADMIN_RO(perf_stats);
>  
>  static ssize_t flags_show(struct device *dev,
>  			  struct device_attribute *attr, char *buf)
> -- 
> 2.26.2
>