linux-kernel - Re: [PATCH] regmap: Add Kconfig option for debugfs register writes

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]

Message-ID: <alpine.DEB.2.02.1406171254540.18156@chino.kir.corp.google.com>
Date:	Tue, 17 Jun 2014 12:56:54 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Markus Pargmann <mpa@...gutronix.de>
cc:	Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, kernel@...gutronix.de,
	Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH] regmap: Add Kconfig option for debugfs register writes

On Tue, 17 Jun 2014, Markus Pargmann wrote:

> To enable writing of registers through the regmap debugfs interface, it
> was necessary to alter the regmap-debugfs.c source code. This is not
> really practical.
> 
> As this is a powerful tool for debugging, this patch creates an expert
> kconfig option for this function. This makes it easier to enable
> register writing in the kernel.
> 
> This patch also fixes the file mode when the regmap debugfs write code is
> compiled.
> 

Forcing users to enable CONFIG_EXPERT for this doesn't seem appropriate, 
it enables things by default and has really become a separate config of 
its own.  You might want to consider removing this dependency unless you 
have a strong reason for keeping it.

> Cc: Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>
> Signed-off-by: Markus Pargmann <mpa@...gutronix.de>
> ---
>  drivers/base/regmap/Kconfig          | 14 ++++++++++++++
>  drivers/base/regmap/regmap-debugfs.c | 16 ++++++++++------
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 4251570610c9..ce86600dbe75 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -23,3 +23,17 @@ config REGMAP_MMIO
>  
>  config REGMAP_IRQ
>  	bool
> +
> +config REGMAP_ALLOW_WRITE_DEBUGFS
> +	default n
> +	depends on DEBUG_FS
> +	depends on EXPERT
> +	depends on REGMAP
> +	bool "Regmap, allow direct register writes through debugfs"
> +	help
> +	  This option enables regmap debugfs write support. If this is enabled,
> +	  you can directly write registers using the registers file in the
> +	  regmap debugfs.
> +
> +	  This is potentially dangerous. Do not enable this option unless you
> +	  know what you are doing.
> diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
> index 45d812c0ea77..12906f2bdf00 100644
> --- a/drivers/base/regmap/regmap-debugfs.c
> +++ b/drivers/base/regmap/regmap-debugfs.c
> @@ -261,13 +261,10 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
>  				   count, ppos);
>  }
>  
> -#undef REGMAP_ALLOW_WRITE_DEBUGFS
> -#ifdef REGMAP_ALLOW_WRITE_DEBUGFS
> +#if IS_ENABLED(CONFIG_REGMAP_ALLOW_WRITE_DEBUGFS)
>  /*
>   * This can be dangerous especially when we have clients such as
> - * PMICs, therefore don't provide any real compile time configuration option
> - * for this feature, people who want to use this will need to modify
> - * the source code directly.
> + * PMICs, therefore this function can only be selected in expert kconfig mode.
>   */
>  static ssize_t regmap_map_write_file(struct file *file,
>  				     const char __user *user_buf,
> @@ -512,7 +509,14 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
>  			    map, &regmap_reg_ranges_fops);
>  
>  	if (map->max_register || regmap_readable(map, 0)) {
> -		debugfs_create_file("registers", 0400, map->debugfs,
> +		unsigned int registers_mode;
> +
> +		if (IS_ENABLED(CONFIG_REGMAP_ALLOW_WRITE_DEBUGFS))
> +			registers_mode = 0600;
> +		else
> +			registers_mode = 0400;
> +
> +		debugfs_create_file("registers", registers_mode, map->debugfs,
>  				    map, &regmap_map_fops);
>  		debugfs_create_file("access", 0400, map->debugfs,
>  				    map, &regmap_access_fops);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/