[PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[Stephen Boyd]

There doesn't seem to be any reason to limit this to only root user
readable. Let's make it readable by all so that random programs can
read the debugfs files in here instead of just root. The information is
just that, informational, so this is fine.

Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/soc/qcom/socinfo.c | 40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index b44ede48decc..4e4946d9e4ab 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -258,7 +258,7 @@ static const struct file_operations qcom_ ##name## _ops = {		\
 }
 
 #define DEBUGFS_ADD(info, name)						\
-	debugfs_create_file(__stringify(name), 0400,			\
+	debugfs_create_file(__stringify(name), 0444,			\
 			    qcom_socinfo->dbg_root,			\
 			    info, &qcom_ ##name## _ops)
 
@@ -349,14 +349,14 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 
 	qcom_socinfo->info.fmt = __le32_to_cpu(info->fmt);
 
-	debugfs_create_x32("info_fmt", 0400, qcom_socinfo->dbg_root,
+	debugfs_create_x32("info_fmt", 0444, qcom_socinfo->dbg_root,
 			   &qcom_socinfo->info.fmt);
 
 	switch (qcom_socinfo->info.fmt) {
 	case SOCINFO_VERSION(0, 15):
 		qcom_socinfo->info.nmodem_supported = __le32_to_cpu(info->nmodem_supported);
 
-		debugfs_create_u32("nmodem_supported", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("nmodem_supported", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.nmodem_supported);
 		fallthrough;
 	case SOCINFO_VERSION(0, 14):
@@ -365,19 +365,19 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 		qcom_socinfo->info.num_defective_parts = __le32_to_cpu(info->num_defective_parts);
 		qcom_socinfo->info.ndefective_parts_array_offset = __le32_to_cpu(info->ndefective_parts_array_offset);
 
-		debugfs_create_u32("num_clusters", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("num_clusters", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.num_clusters);
-		debugfs_create_u32("ncluster_array_offset", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("ncluster_array_offset", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.ncluster_array_offset);
-		debugfs_create_u32("num_defective_parts", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("num_defective_parts", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.num_defective_parts);
-		debugfs_create_u32("ndefective_parts_array_offset", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("ndefective_parts_array_offset", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.ndefective_parts_array_offset);
 		fallthrough;
 	case SOCINFO_VERSION(0, 13):
 		qcom_socinfo->info.nproduct_id = __le32_to_cpu(info->nproduct_id);
 
-		debugfs_create_u32("nproduct_id", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("nproduct_id", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.nproduct_id);
 		DEBUGFS_ADD(info, chip_id);
 		fallthrough;
@@ -389,12 +389,12 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 		qcom_socinfo->info.raw_device_num =
 			__le32_to_cpu(info->raw_device_num);
 
-		debugfs_create_x32("chip_family", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_x32("chip_family", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.chip_family);
-		debugfs_create_x32("raw_device_family", 0400,
+		debugfs_create_x32("raw_device_family", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.raw_device_family);
-		debugfs_create_x32("raw_device_number", 0400,
+		debugfs_create_x32("raw_device_number", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.raw_device_num);
 		fallthrough;
@@ -403,7 +403,7 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 	case SOCINFO_VERSION(0, 9):
 		qcom_socinfo->info.foundry_id = __le32_to_cpu(info->foundry_id);
 
-		debugfs_create_u32("foundry_id", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("foundry_id", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.foundry_id);
 		fallthrough;
 	case SOCINFO_VERSION(0, 8):
@@ -415,7 +415,7 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 		qcom_socinfo->info.hw_plat_subtype =
 			__le32_to_cpu(info->hw_plat_subtype);
 
-		debugfs_create_u32("hardware_platform_subtype", 0400,
+		debugfs_create_u32("hardware_platform_subtype", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.hw_plat_subtype);
 		fallthrough;
@@ -423,28 +423,28 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 		qcom_socinfo->info.accessory_chip =
 			__le32_to_cpu(info->accessory_chip);
 
-		debugfs_create_u32("accessory_chip", 0400,
+		debugfs_create_u32("accessory_chip", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.accessory_chip);
 		fallthrough;
 	case SOCINFO_VERSION(0, 4):
 		qcom_socinfo->info.plat_ver = __le32_to_cpu(info->plat_ver);
 
-		debugfs_create_u32("platform_version", 0400,
+		debugfs_create_u32("platform_version", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.plat_ver);
 		fallthrough;
 	case SOCINFO_VERSION(0, 3):
 		qcom_socinfo->info.hw_plat = __le32_to_cpu(info->hw_plat);
 
-		debugfs_create_u32("hardware_platform", 0400,
+		debugfs_create_u32("hardware_platform", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.hw_plat);
 		fallthrough;
 	case SOCINFO_VERSION(0, 2):
 		qcom_socinfo->info.raw_ver  = __le32_to_cpu(info->raw_ver);
 
-		debugfs_create_u32("raw_version", 0400, qcom_socinfo->dbg_root,
+		debugfs_create_u32("raw_version", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.raw_ver);
 		fallthrough;
 	case SOCINFO_VERSION(0, 1):
@@ -461,11 +461,11 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 
 		dentry = debugfs_create_dir(socinfo_image_names[i],
 					    qcom_socinfo->dbg_root);
-		debugfs_create_file("name", 0400, dentry, &versions[i],
+		debugfs_create_file("name", 0444, dentry, &versions[i],
 				    &qcom_image_name_ops);
-		debugfs_create_file("variant", 0400, dentry, &versions[i],
+		debugfs_create_file("variant", 0444, dentry, &versions[i],
 				    &qcom_image_variant_ops);
-		debugfs_create_file("oem", 0400, dentry, &versions[i],
+		debugfs_create_file("oem", 0444, dentry, &versions[i],
 				    &qcom_image_oem_ops);
 	}
 }

base-commit: 2c85ebc57b3e1817b6ce1a6b703928e113a90442
-- 
https://chromeos.dev

Re: [PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[Sai Prakash Ranjan]

On 2020-12-16 12:49, Stephen Boyd wrote:
> There doesn't seem to be any reason to limit this to only root user
> readable. Let's make it readable by all so that random programs can
> read the debugfs files in here instead of just root. The information is
> just that, informational, so this is fine.
> 
> Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/soc/qcom/socinfo.c | 40 +++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index b44ede48decc..4e4946d9e4ab 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -258,7 +258,7 @@ static const struct file_operations qcom_ ##name##
> _ops = {		\
>  }
> 
>  #define DEBUGFS_ADD(info, name)						\
> -	debugfs_create_file(__stringify(name), 0400,			\
> +	debugfs_create_file(__stringify(name), 0444,			\
>  			    qcom_socinfo->dbg_root,			\
>  			    info, &qcom_ ##name## _ops)
> 
> @@ -349,14 +349,14 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
> 
>  	qcom_socinfo->info.fmt = __le32_to_cpu(info->fmt);
> 
> -	debugfs_create_x32("info_fmt", 0400, qcom_socinfo->dbg_root,
> +	debugfs_create_x32("info_fmt", 0444, qcom_socinfo->dbg_root,
>  			   &qcom_socinfo->info.fmt);
> 
>  	switch (qcom_socinfo->info.fmt) {
>  	case SOCINFO_VERSION(0, 15):
>  		qcom_socinfo->info.nmodem_supported = 
> __le32_to_cpu(info->nmodem_supported);
> 
> -		debugfs_create_u32("nmodem_supported", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_u32("nmodem_supported", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.nmodem_supported);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 14):
> @@ -365,19 +365,19 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
>  		qcom_socinfo->info.num_defective_parts =
> __le32_to_cpu(info->num_defective_parts);
>  		qcom_socinfo->info.ndefective_parts_array_offset =
> __le32_to_cpu(info->ndefective_parts_array_offset);
> 
> -		debugfs_create_u32("num_clusters", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_u32("num_clusters", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.num_clusters);
> -		debugfs_create_u32("ncluster_array_offset", 0400, 
> qcom_socinfo->dbg_root,
> +		debugfs_create_u32("ncluster_array_offset", 0444, 
> qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.ncluster_array_offset);
> -		debugfs_create_u32("num_defective_parts", 0400, 
> qcom_socinfo->dbg_root,
> +		debugfs_create_u32("num_defective_parts", 0444, 
> qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.num_defective_parts);
> -		debugfs_create_u32("ndefective_parts_array_offset", 0400,
> qcom_socinfo->dbg_root,
> +		debugfs_create_u32("ndefective_parts_array_offset", 0444,
> qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.ndefective_parts_array_offset);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 13):
>  		qcom_socinfo->info.nproduct_id = __le32_to_cpu(info->nproduct_id);
> 
> -		debugfs_create_u32("nproduct_id", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_u32("nproduct_id", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.nproduct_id);
>  		DEBUGFS_ADD(info, chip_id);
>  		fallthrough;
> @@ -389,12 +389,12 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
>  		qcom_socinfo->info.raw_device_num =
>  			__le32_to_cpu(info->raw_device_num);
> 
> -		debugfs_create_x32("chip_family", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_x32("chip_family", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.chip_family);
> -		debugfs_create_x32("raw_device_family", 0400,
> +		debugfs_create_x32("raw_device_family", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.raw_device_family);
> -		debugfs_create_x32("raw_device_number", 0400,
> +		debugfs_create_x32("raw_device_number", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.raw_device_num);
>  		fallthrough;
> @@ -403,7 +403,7 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
>  	case SOCINFO_VERSION(0, 9):
>  		qcom_socinfo->info.foundry_id = __le32_to_cpu(info->foundry_id);
> 
> -		debugfs_create_u32("foundry_id", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_u32("foundry_id", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.foundry_id);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 8):
> @@ -415,7 +415,7 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
>  		qcom_socinfo->info.hw_plat_subtype =
>  			__le32_to_cpu(info->hw_plat_subtype);
> 
> -		debugfs_create_u32("hardware_platform_subtype", 0400,
> +		debugfs_create_u32("hardware_platform_subtype", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.hw_plat_subtype);
>  		fallthrough;
> @@ -423,28 +423,28 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
>  		qcom_socinfo->info.accessory_chip =
>  			__le32_to_cpu(info->accessory_chip);
> 
> -		debugfs_create_u32("accessory_chip", 0400,
> +		debugfs_create_u32("accessory_chip", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.accessory_chip);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 4):
>  		qcom_socinfo->info.plat_ver = __le32_to_cpu(info->plat_ver);
> 
> -		debugfs_create_u32("platform_version", 0400,
> +		debugfs_create_u32("platform_version", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.plat_ver);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 3):
>  		qcom_socinfo->info.hw_plat = __le32_to_cpu(info->hw_plat);
> 
> -		debugfs_create_u32("hardware_platform", 0400,
> +		debugfs_create_u32("hardware_platform", 0444,
>  				   qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.hw_plat);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 2):
>  		qcom_socinfo->info.raw_ver  = __le32_to_cpu(info->raw_ver);
> 
> -		debugfs_create_u32("raw_version", 0400, qcom_socinfo->dbg_root,
> +		debugfs_create_u32("raw_version", 0444, qcom_socinfo->dbg_root,
>  				   &qcom_socinfo->info.raw_ver);
>  		fallthrough;
>  	case SOCINFO_VERSION(0, 1):
> @@ -461,11 +461,11 @@ static void socinfo_debugfs_init(struct
> qcom_socinfo *qcom_socinfo,
> 
>  		dentry = debugfs_create_dir(socinfo_image_names[i],
>  					    qcom_socinfo->dbg_root);
> -		debugfs_create_file("name", 0400, dentry, &versions[i],
> +		debugfs_create_file("name", 0444, dentry, &versions[i],
>  				    &qcom_image_name_ops);
> -		debugfs_create_file("variant", 0400, dentry, &versions[i],
> +		debugfs_create_file("variant", 0444, dentry, &versions[i],
>  				    &qcom_image_variant_ops);
> -		debugfs_create_file("oem", 0400, dentry, &versions[i],
> +		debugfs_create_file("oem", 0444, dentry, &versions[i],
>  				    &qcom_image_oem_ops);
>  	}
>  }
> 
> base-commit: 2c85ebc57b3e1817b6ce1a6b703928e113a90442

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[Doug Anderson]

Hi,

On Tue, Dec 15, 2020 at 11:19 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> There doesn't seem to be any reason to limit this to only root user
> readable. Let's make it readable by all so that random programs can
> read the debugfs files in here instead of just root. The information is
> just that, informational, so this is fine.
>
> Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/soc/qcom/socinfo.c | 40 +++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)

One worry I'd have is whether there would ever be any PII (personally
identifiable information) here, like maybe a chip serial number.  If
so, is that something that is OK to provide to any random process?
...maybe I'm just being paranoid though, since presumably there are
enough random HW characteristics that could be strung together and
hashed to make roughly a unique ID anyway and hiding every HW
characteristic would be a bit extreme...

-Doug

Re: [PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[Stephen Boyd]

Quoting Doug Anderson (2021-01-05 15:06:35)
> Hi,
> 
> On Tue, Dec 15, 2020 at 11:19 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > There doesn't seem to be any reason to limit this to only root user
> > readable. Let's make it readable by all so that random programs can
> > read the debugfs files in here instead of just root. The information is
> > just that, informational, so this is fine.
> >
> > Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  drivers/soc/qcom/socinfo.c | 40 +++++++++++++++++++-------------------
> >  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> One worry I'd have is whether there would ever be any PII (personally
> identifiable information) here, like maybe a chip serial number.  If
> so, is that something that is OK to provide to any random process?
> ...maybe I'm just being paranoid though, since presumably there are
> enough random HW characteristics that could be strung together and
> hashed to make roughly a unique ID anyway and hiding every HW
> characteristic would be a bit extreme...
> 

I suppose if that's a problem then the process that mounts debugfs can
change the access to restrict it. I'm not aware of this needing to be a
kernel policy though, so I'd rather leave that up to userspace to decide
if it should be restricted further.

Re: [PATCH] soc: qcom: socinfo: Open read access to all for debugfs

[Doug Anderson]

Hi,

On Mon, Jan 11, 2021 at 6:04 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Doug Anderson (2021-01-05 15:06:35)
> > Hi,
> >
> > On Tue, Dec 15, 2020 at 11:19 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > There doesn't seem to be any reason to limit this to only root user
> > > readable. Let's make it readable by all so that random programs can
> > > read the debugfs files in here instead of just root. The information is
> > > just that, informational, so this is fine.
> > >
> > > Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > Cc: Douglas Anderson <dianders@chromium.org>
> > > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > >  drivers/soc/qcom/socinfo.c | 40 +++++++++++++++++++-------------------
> > >  1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > One worry I'd have is whether there would ever be any PII (personally
> > identifiable information) here, like maybe a chip serial number.  If
> > so, is that something that is OK to provide to any random process?
> > ...maybe I'm just being paranoid though, since presumably there are
> > enough random HW characteristics that could be strung together and
> > hashed to make roughly a unique ID anyway and hiding every HW
> > characteristic would be a bit extreme...
> >
>
> I suppose if that's a problem then the process that mounts debugfs can
> change the access to restrict it. I'm not aware of this needing to be a
> kernel policy though, so I'd rather leave that up to userspace to decide
> if it should be restricted further.

OK, fair enough.

Reviewed-by: Douglas Anderson <dianders@chromium.org>