Current nvmem framework allows user space to read all register space
populated by nvmem binary file, In case we don't want to expose value
of registers to userspace and only want kernel space to read cell
value from nvmem_cell_read_u32.
To protect the same, Add no-read-write property to prevent read
from userspace.
Signed-off-by: Gaurav Kohli <[email protected]>
---
v2: Fix build error
v1: Fix no_read_write update condition
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index f24008b..f1c44fc 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -27,6 +27,7 @@ struct nvmem_device {
struct kref refcnt;
size_t size;
bool read_only;
+ bool no_read_write;
int flags;
enum nvmem_type type;
struct bin_attribute eeprom;
@@ -120,6 +121,10 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
dev = container_of(kobj, struct device, kobj);
nvmem = to_nvmem_device(dev);
+ /* if no-read-write, then stop from reading */
+ if (nvmem->no_read_write)
+ return -EPERM;
+
/* Stop the user from reading */
if (pos >= nvmem->size)
return 0;
@@ -154,6 +159,10 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
dev = container_of(kobj, struct device, kobj);
nvmem = to_nvmem_device(dev);
+ /* if no-read-write, then stop from writing */
+ if (nvmem->no_read_write)
+ return -EPERM;
+
/* Stop the user from writing */
if (pos >= nvmem->size)
return -EFBIG;
@@ -651,6 +660,9 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->read_only = device_property_present(config->dev, "read-only") ||
config->read_only || !nvmem->reg_write;
+ nvmem->no_read_write = device_property_present(config->dev,
+ "no-read-write");
+
if (config->root_only)
nvmem->dev.groups = nvmem->read_only ?
nvmem_ro_root_dev_groups :
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On 17/03/2019 14:12, Gaurav Kohli wrote:
> Current nvmem framework allows user space to read all register space
> populated by nvmem binary file, In case we don't want to expose value
> of registers to userspace and only want kernel space to read cell
> value from nvmem_cell_read_u32.
>
> To protect the same, Add no-read-write property to prevent read
> from userspace.
>
Can you explain the real need of this?
Is there any issue you are noticing while reading nvmem content from
userspace?
I don't think this is the right way to do this, its misleading in many
ways. Also this should not be a part of DT binding.
If we decide that we need this feature, then better way to do this using
a new Kernel config.
thanks,
srini
On 3/20/2019 8:04 PM, Srinivas Kandagatla wrote:
>
>
> On 17/03/2019 14:12, Gaurav Kohli wrote:
>> Current nvmem framework allows user space to read all register space
>> populated by nvmem binary file, In case we don't want to expose value
>> of registers to userspace and only want kernel space to read cell
>> value from nvmem_cell_read_u32.
>>
>> To protect the same, Add no-read-write property to prevent read
>> from userspace.
>>
>
> Can you explain the real need of this?
> Is there any issue you are noticing while reading nvmem content from
> userspace?
>
Hi Srinivas,
No, We are not observing any issue, nvmem is dumping the data properly.
But there are certain register, which we don't want to expose to user
space and want kernel space can only read via nvmem_cell_read.
In existing design, even if we read cell from kernel space, nvmem binary
files is still populated to user space unconditionally.
Regards
Gaurav
> I don't think this is the right way to do this, its misleading in many
> ways. Also this should not be a part of DT binding.
>
> If we decide that we need this feature, then better way to do this
> using a new Kernel config.
>
> thanks,
> srini
-- Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project.
On 20/03/2019 15:50, Gaurav Kohli wrote:
>
> On 3/20/2019 8:04 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 17/03/2019 14:12, Gaurav Kohli wrote:
>>> Current nvmem framework allows user space to read all register space
>>> populated by nvmem binary file, In case we don't want to expose value
>>> of registers to userspace and only want kernel space to read cell
>>> value from nvmem_cell_read_u32.
>>>
>>> To protect the same, Add no-read-write property to prevent read
>>> from userspace.
>>>
>>
>> Can you explain the real need of this?
>> Is there any issue you are noticing while reading nvmem content from
>> userspace?
>>
> Hi Srinivas,
>
>
> No, We are not observing any issue, nvmem is dumping the data properly.
>
> But there are certain register, which we don't want to expose to user
> space and want kernel space can only read via nvmem_cell_read.
Am guessing these are some kind of keys or something that you do not
want user to see.
Is root only option not helping you in this case?
We could go down the route of adding new config option something like
CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in userspace.
Let me know if you are happy to create a patch for this change?
Thanks,
srini
>
> In existing design, even if we read cell from kernel space, nvmem binary
> files is still populated to user space unconditionally.
>
> Regards
>
> Gaurav
On 3/20/2019 9:56 PM, Srinivas Kandagatla wrote:
>
>
> On 20/03/2019 15:50, Gaurav Kohli wrote:
>>
>> On 3/20/2019 8:04 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 17/03/2019 14:12, Gaurav Kohli wrote:
>>>> Current nvmem framework allows user space to read all register space
>>>> populated by nvmem binary file, In case we don't want to expose value
>>>> of registers to userspace and only want kernel space to read cell
>>>> value from nvmem_cell_read_u32.
>>>>
>>>> To protect the same, Add no-read-write property to prevent read
>>>> from userspace.
>>>>
>>>
>>> Can you explain the real need of this?
>>> Is there any issue you are noticing while reading nvmem content from
>>> userspace?
>>>
>> Hi Srinivas,
>>
>>
>> No, We are not observing any issue, nvmem is dumping the data properly.
>>
>> But there are certain register, which we don't want to expose to user
>> space and want kernel space can only read via nvmem_cell_read.
> Am guessing these are some kind of keys or something that you do not
> want user to see.
>
Hi Srinivas,
Thanks.
Yes exactly, there are certain keys or even certain bit that we don't
want to expose to user.
> Is root only option not helping you in this case?
Yes we want to protect at root level as well, i mean it is better if we
can avoid exposing to userspace at all.
>
> We could go down the route of adding new config option something like
> CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in userspace.
>
> Let me know if you are happy to create a patch for this change?
I am happy with either way config option or dt binding(seems easy),
please let me know we will post new patch for the same.
In config option, do i have to remove all binary creation code of nvmem
, correct? or simply put a check like dt binding option
so user cannot read it.
Thanks
Gaurav
>
> Thanks,
> srini
>
>>
>> In existing design, even if we read cell from kernel space, nvmem
>> binary files is still populated to user space unconditionally.
>>
>> Regards
>>
>> Gaurav
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On 20/03/2019 18:50, Gaurav Kohli wrote:
> On 3/20/2019 9:56 PM, Srinivas Kandagatla wrote:
>
>> Am guessing these are some kind of keys or something that you do not
>> want user to see.
>
> Yes exactly, there are certain keys or even certain bit that we don't
> want to expose to user.
>
>> Is root only option not helping you in this case?
>
> Yes we want to protect at root level as well, I mean it is better if we
> can avoid exposing to userspace at all.
NB: root has access to /dev/mem and /dev/kmem (and probably other ways
into the kernel)
>> We could go down the route of adding new config option something like
>> CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in userspace.
>>
>> Let me know if you are happy to create a patch for this change?
>
> I am happy with either way, config option or DT binding (seems easy),
> please let me know we will post new patch for the same.
Device tree nodes are supposed to be hardware descriptions. Obviously,
implementing security policies such as "hide this information from
user-space" is not a good fit for a DT node.
The qcom kernel is full of "config knobs" in DT nodes. It would be good
if you could spread the word, and slowly turn things around :-)
Regards.
On 20/03/2019 17:50, Gaurav Kohli wrote:
>
>> Is root only option not helping you in this case?
> Yes we want to protect at root level as well, i mean it is better if we
> can avoid exposing to userspace at all.
Can you try below patch!
>>
>> We could go down the route of adding new config option something like
>> CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in userspace.
>>
>> Let me know if you are happy to create a patch for this change?
>
> I am happy with either way config option or dt binding(seems easy),
> please let me know we will post new patch for the same.
DT way is totally NAK.
--------------------------->cut<-----------------------------------
From: Srinivas Kandagatla <[email protected]>
Date: Wed, 20 Mar 2019 16:15:21 +0000
Subject: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY
Some users might not want to expose nvmem entry to sysfs and
only intend to use kernel interface so add such provision.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
Documentation/ABI/stable/sysfs-bus-nvmem | 2 ++
drivers/nvmem/Kconfig | 5 +++++
drivers/nvmem/core.c | 11 ++++++-----
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
b/Documentation/ABI/stable/sysfs-bus-nvmem
index 5923ab4620c5..12aab0a85fea 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -6,6 +6,8 @@ Description:
This file allows user to read/write the raw NVMEM contents.
Permissions for write to this file depends on the nvmem
provider configuration.
+ Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
+ is enabled
ex:
hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 0a7a470ee859..6ab3276d287c 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -192,4 +192,9 @@ config SC27XX_EFUSE
This driver can also be built as a module. If so, the module
will be called nvmem-sc27xx-efuse.
+config NVMEM_NO_SYSFS_ENTRY
+ bool "No nvmem sysfs entry"
+
+ help
+ Say Yes if you do not want to add nvmem entry to sysfs.
endif
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b9a0270883a0..c70f183fe379 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -216,7 +216,7 @@ static const struct attribute_group
nvmem_bin_rw_group = {
.attrs = nvmem_attrs,
};
-static const struct attribute_group *nvmem_rw_dev_groups[] = {
+static const __maybe_unused struct attribute_group
*nvmem_rw_dev_groups[] = {
&nvmem_bin_rw_group,
NULL,
};
@@ -240,7 +240,7 @@ static const struct attribute_group
nvmem_bin_ro_group = {
.attrs = nvmem_attrs,
};
-static const struct attribute_group *nvmem_ro_dev_groups[] = {
+static const __maybe_unused struct attribute_group
*nvmem_ro_dev_groups[] = {
&nvmem_bin_ro_group,
NULL,
};
@@ -265,7 +265,7 @@ static const struct attribute_group
nvmem_bin_rw_root_group = {
.attrs = nvmem_attrs,
};
-static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group
*nvmem_rw_root_dev_groups[] = {
&nvmem_bin_rw_root_group,
NULL,
};
@@ -289,7 +289,7 @@ static const struct attribute_group
nvmem_bin_ro_root_group = {
.attrs = nvmem_attrs,
};
-static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group
*nvmem_ro_root_dev_groups[] = {
&nvmem_bin_ro_root_group,
NULL,
};
@@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->read_only = device_property_present(config->dev, "read-only") |
config->read_only;
+#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
if (config->root_only)
nvmem->dev.groups = nvmem->read_only ?
nvmem_ro_root_dev_groups :
@@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->dev.groups = nvmem->read_only ?
nvmem_ro_dev_groups :
nvmem_rw_dev_groups;
-
+#endif
device_initialize(&nvmem->dev);
dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
--
2.21.0
--------------------------->cut<-----------------------------------
Hi Srinivas,
Thanks for the patch, Something like this only i have tested in the
morning, instead of unused, i have put dev group inside config as well.
We will test the exact patch and update the same.
Regards
Gaurav
On 3/22/2019 8:32 PM, Srinivas Kandagatla wrote:
>
>
> On 20/03/2019 17:50, Gaurav Kohli wrote:
>>
>>> Is root only option not helping you in this case?
>> Yes we want to protect at root level as well, i mean it is better if
>> we can avoid exposing to userspace at all.
> Can you try below patch!
>
>>>
>>> We could go down the route of adding new config option something
>>> like CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in
>>> userspace.
>>>
>>> Let me know if you are happy to create a patch for this change?
>>
>> I am happy with either way config option or dt binding(seems easy),
>> please let me know we will post new patch for the same.
> DT way is totally NAK.
>
>
> --------------------------->cut<-----------------------------------
>
> From: Srinivas Kandagatla <[email protected]>
> Date: Wed, 20 Mar 2019 16:15:21 +0000
> Subject: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY
>
> Some users might not want to expose nvmem entry to sysfs and
> only intend to use kernel interface so add such provision.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> Documentation/ABI/stable/sysfs-bus-nvmem | 2 ++
> drivers/nvmem/Kconfig | 5 +++++
> drivers/nvmem/core.c | 11 ++++++-----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
> b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..12aab0a85fea 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
> This file allows user to read/write the raw NVMEM contents.
> Permissions for write to this file depends on the nvmem
> provider configuration.
> + Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
> + is enabled
>
> ex:
> hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 0a7a470ee859..6ab3276d287c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
> This driver can also be built as a module. If so, the module
> will be called nvmem-sc27xx-efuse.
>
> +config NVMEM_NO_SYSFS_ENTRY
> + bool "No nvmem sysfs entry"
> +
> + help
> + Say Yes if you do not want to add nvmem entry to sysfs.
> endif
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index b9a0270883a0..c70f183fe379 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -216,7 +216,7 @@ static const struct attribute_group
> nvmem_bin_rw_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_dev_groups[] = {
> &nvmem_bin_rw_group,
> NULL,
> };
> @@ -240,7 +240,7 @@ static const struct attribute_group
> nvmem_bin_ro_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_dev_groups[] = {
> &nvmem_bin_ro_group,
> NULL,
> };
> @@ -265,7 +265,7 @@ static const struct attribute_group
> nvmem_bin_rw_root_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_root_dev_groups[] = {
> &nvmem_bin_rw_root_group,
> NULL,
> };
> @@ -289,7 +289,7 @@ static const struct attribute_group
> nvmem_bin_ro_root_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_root_dev_groups[] = {
> &nvmem_bin_ro_root_group,
> NULL,
> };
> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
> nvmem->read_only = device_property_present(config->dev,
> "read-only") |
> config->read_only;
>
> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
> if (config->root_only)
> nvmem->dev.groups = nvmem->read_only ?
> nvmem_ro_root_dev_groups :
> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
> nvmem->dev.groups = nvmem->read_only ?
> nvmem_ro_dev_groups :
> nvmem_rw_dev_groups;
> -
> +#endif
> device_initialize(&nvmem->dev);
>
> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On 3/22/2019 11:42 PM, Gaurav Kohli wrote:
> Hi Srinivas,
>
> Thanks for the patch, Something like this only i have tested in the
> morning, instead of unused, i have put dev group inside config as well.
>
> We will test the exact patch and update the same.
>
> Regards
>
> Gaurav
>
> On 3/22/2019 8:32 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 20/03/2019 17:50, Gaurav Kohli wrote:
>>>
>>>> Is root only option not helping you in this case?
>>> Yes we want to protect at root level as well, i mean it is better if
>>> we can avoid exposing to userspace at all.
>> Can you try below patch!
>>
Hi Srinivas,
Thanks for the patch.
I have checked the patch and also tested the same , and it is working as
expected(no creation of nvmem sysfs)
Applied with some conflict on line 41.
Reviewed-by: Gaurav Kohli <[email protected]>
Tested-by: Gaurav Kohli <[email protected]>
Regards
Gaurav
>>>>
>>>> We could go down the route of adding new config option something
>>>> like CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in
>>>> userspace.
>>>>
>>>> Let me know if you are happy to create a patch for this change?
>>>
>>> I am happy with either way config option or dt binding(seems easy),
>>> please let me know we will post new patch for the same.
>> DT way is totally NAK.
>>
>>
>> --------------------------->cut<-----------------------------------
>>
>> From: Srinivas Kandagatla <[email protected]>
>> Date: Wed, 20 Mar 2019 16:15:21 +0000
>> Subject: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY
>>
>> Some users might not want to expose nvmem entry to sysfs and
>> only intend to use kernel interface so add such provision.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> Documentation/ABI/stable/sysfs-bus-nvmem | 2 ++
>> drivers/nvmem/Kconfig | 5 +++++
>> drivers/nvmem/core.c | 11 ++++++-----
>> 3 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
>> b/Documentation/ABI/stable/sysfs-bus-nvmem
>> index 5923ab4620c5..12aab0a85fea 100644
>> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
>> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
>> @@ -6,6 +6,8 @@ Description:
>> This file allows user to read/write the raw NVMEM contents.
>> Permissions for write to this file depends on the nvmem
>> provider configuration.
>> + Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
>> + is enabled
>>
>> ex:
>> hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index 0a7a470ee859..6ab3276d287c 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
>> This driver can also be built as a module. If so, the module
>> will be called nvmem-sc27xx-efuse.
>>
>> +config NVMEM_NO_SYSFS_ENTRY
>> + bool "No nvmem sysfs entry"
>> +
>> + help
>> + Say Yes if you do not want to add nvmem entry to sysfs.
>> endif
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index b9a0270883a0..c70f183fe379 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -216,7 +216,7 @@ static const struct attribute_group
>> nvmem_bin_rw_group = {
>> .attrs = nvmem_attrs,
>> };
>>
>> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
>> +static const __maybe_unused struct attribute_group
>> *nvmem_rw_dev_groups[] = {
>> &nvmem_bin_rw_group,
>> NULL,
>> };
>> @@ -240,7 +240,7 @@ static const struct attribute_group
>> nvmem_bin_ro_group = {
>> .attrs = nvmem_attrs,
>> };
>>
>> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
>> +static const __maybe_unused struct attribute_group
>> *nvmem_ro_dev_groups[] = {
>> &nvmem_bin_ro_group,
>> NULL,
>> };
>> @@ -265,7 +265,7 @@ static const struct attribute_group
>> nvmem_bin_rw_root_group = {
>> .attrs = nvmem_attrs,
>> };
>>
>> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
>> +static const __maybe_unused struct attribute_group
>> *nvmem_rw_root_dev_groups[] = {
>> &nvmem_bin_rw_root_group,
>> NULL,
>> };
>> @@ -289,7 +289,7 @@ static const struct attribute_group
>> nvmem_bin_ro_root_group = {
>> .attrs = nvmem_attrs,
>> };
>>
>> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
>> +static const __maybe_unused struct attribute_group
>> *nvmem_ro_root_dev_groups[] = {
>> &nvmem_bin_ro_root_group,
>> NULL,
>> };
>> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
>> nvmem_config *config)
>> nvmem->read_only = device_property_present(config->dev,
>> "read-only") |
>> config->read_only;
>>
>> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
>> if (config->root_only)
>> nvmem->dev.groups = nvmem->read_only ?
>> nvmem_ro_root_dev_groups :
>> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
>> nvmem_config *config)
>> nvmem->dev.groups = nvmem->read_only ?
>> nvmem_ro_dev_groups :
>> nvmem_rw_dev_groups;
>> -
>> +#endif
>> device_initialize(&nvmem->dev);
>>
>> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
>> config->name);
>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
Hi Srinivas,
Thanks for the patch, Can you please merge , So will cherry pick the same.
Regards
Gaurav
On 3/25/2019 11:45 AM, Gaurav Kohli wrote:
>
> On 3/22/2019 11:42 PM, Gaurav Kohli wrote:
>> Hi Srinivas,
>>
>> Thanks for the patch, Something like this only i have tested in the
>> morning, instead of unused, i have put dev group inside config as well.
>>
>> We will test the exact patch and update the same.
>>
>> Regards
>>
>> Gaurav
>>
>> On 3/22/2019 8:32 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 20/03/2019 17:50, Gaurav Kohli wrote:
>>>>
>>>>> Is root only option not helping you in this case?
>>>> Yes we want to protect at root level as well, i mean it is better
>>>> if we can avoid exposing to userspace at all.
>>> Can you try below patch!
>>>
> Hi Srinivas,
>
> Thanks for the patch.
>
> I have checked the patch and also tested the same , and it is working
> as expected(no creation of nvmem sysfs)
>
> Applied with some conflict on line 41.
>
> Reviewed-by: Gaurav Kohli <[email protected]>
> Tested-by: Gaurav Kohli <[email protected]>
>
> Regards
>
> Gaurav
>
>
>>>>>
>>>>> We could go down the route of adding new config option something
>>>>> like CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in
>>>>> userspace.
>>>>>
>>>>> Let me know if you are happy to create a patch for this change?
>>>>
>>>> I am happy with either way config option or dt binding(seems easy),
>>>> please let me know we will post new patch for the same.
>>> DT way is totally NAK.
>>>
>>>
>>> --------------------------->cut<-----------------------------------
>>>
>>> From: Srinivas Kandagatla <[email protected]>
>>> Date: Wed, 20 Mar 2019 16:15:21 +0000
>>> Subject: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY
>>>
>>> Some users might not want to expose nvmem entry to sysfs and
>>> only intend to use kernel interface so add such provision.
>>>
>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>> ---
>>> Documentation/ABI/stable/sysfs-bus-nvmem | 2 ++
>>> drivers/nvmem/Kconfig | 5 +++++
>>> drivers/nvmem/core.c | 11 ++++++-----
>>> 3 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
>>> b/Documentation/ABI/stable/sysfs-bus-nvmem
>>> index 5923ab4620c5..12aab0a85fea 100644
>>> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
>>> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
>>> @@ -6,6 +6,8 @@ Description:
>>> This file allows user to read/write the raw NVMEM contents.
>>> Permissions for write to this file depends on the nvmem
>>> provider configuration.
>>> + Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
>>> + is enabled
>>>
>>> ex:
>>> hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
>>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>>> index 0a7a470ee859..6ab3276d287c 100644
>>> --- a/drivers/nvmem/Kconfig
>>> +++ b/drivers/nvmem/Kconfig
>>> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
>>> This driver can also be built as a module. If so, the module
>>> will be called nvmem-sc27xx-efuse.
>>>
>>> +config NVMEM_NO_SYSFS_ENTRY
>>> + bool "No nvmem sysfs entry"
>>> +
>>> + help
>>> + Say Yes if you do not want to add nvmem entry to sysfs.
>>> endif
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> index b9a0270883a0..c70f183fe379 100644
>>> --- a/drivers/nvmem/core.c
>>> +++ b/drivers/nvmem/core.c
>>> @@ -216,7 +216,7 @@ static const struct attribute_group
>>> nvmem_bin_rw_group = {
>>> .attrs = nvmem_attrs,
>>> };
>>>
>>> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
>>> +static const __maybe_unused struct attribute_group
>>> *nvmem_rw_dev_groups[] = {
>>> &nvmem_bin_rw_group,
>>> NULL,
>>> };
>>> @@ -240,7 +240,7 @@ static const struct attribute_group
>>> nvmem_bin_ro_group = {
>>> .attrs = nvmem_attrs,
>>> };
>>>
>>> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
>>> +static const __maybe_unused struct attribute_group
>>> *nvmem_ro_dev_groups[] = {
>>> &nvmem_bin_ro_group,
>>> NULL,
>>> };
>>> @@ -265,7 +265,7 @@ static const struct attribute_group
>>> nvmem_bin_rw_root_group = {
>>> .attrs = nvmem_attrs,
>>> };
>>>
>>> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
>>> +static const __maybe_unused struct attribute_group
>>> *nvmem_rw_root_dev_groups[] = {
>>> &nvmem_bin_rw_root_group,
>>> NULL,
>>> };
>>> @@ -289,7 +289,7 @@ static const struct attribute_group
>>> nvmem_bin_ro_root_group = {
>>> .attrs = nvmem_attrs,
>>> };
>>>
>>> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
>>> +static const __maybe_unused struct attribute_group
>>> *nvmem_ro_root_dev_groups[] = {
>>> &nvmem_bin_ro_root_group,
>>> NULL,
>>> };
>>> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
>>> nvmem_config *config)
>>> nvmem->read_only = device_property_present(config->dev,
>>> "read-only") |
>>> config->read_only;
>>>
>>> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
>>> if (config->root_only)
>>> nvmem->dev.groups = nvmem->read_only ?
>>> nvmem_ro_root_dev_groups :
>>> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
>>> nvmem_config *config)
>>> nvmem->dev.groups = nvmem->read_only ?
>>> nvmem_ro_dev_groups :
>>> nvmem_rw_dev_groups;
>>> -
>>> +#endif
>>> device_initialize(&nvmem->dev);
>>>
>>> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
>>> config->name);
>>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On Fri, Mar 22, 2019 at 03:02:11PM +0000, Srinivas Kandagatla wrote:
>
>
> On 20/03/2019 17:50, Gaurav Kohli wrote:
> >
> > > Is root only option not helping you in this case?
> > Yes we want to protect at root level as well, i mean it is better if we
> > can avoid exposing to userspace at all.
> Can you try below patch!
>
> > >
> > > We could go down the route of adding new config option something
> > > like CONFIG_NVMEM_NO_SYSFS_ENTRY to prevent adding nvmem entry in
> > > userspace.
> > >
> > > Let me know if you are happy to create a patch for this change?
> >
> > I am happy with either way config option or dt binding(seems easy),
> > please let me know we will post new patch for the same.
> DT way is totally NAK.
>
>
> --------------------------->cut<-----------------------------------
>
> From: Srinivas Kandagatla <[email protected]>
> Date: Wed, 20 Mar 2019 16:15:21 +0000
> Subject: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY
>
> Some users might not want to expose nvmem entry to sysfs and
> only intend to use kernel interface so add such provision.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> Documentation/ABI/stable/sysfs-bus-nvmem | 2 ++
> drivers/nvmem/Kconfig | 5 +++++
> drivers/nvmem/core.c | 11 ++++++-----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
> b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..12aab0a85fea 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
> This file allows user to read/write the raw NVMEM contents.
> Permissions for write to this file depends on the nvmem
> provider configuration.
> + Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
> + is enabled
>
> ex:
> hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 0a7a470ee859..6ab3276d287c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
> This driver can also be built as a module. If so, the module
> will be called nvmem-sc27xx-efuse.
>
> +config NVMEM_NO_SYSFS_ENTRY
> + bool "No nvmem sysfs entry"
> +
> + help
> + Say Yes if you do not want to add nvmem entry to sysfs.
> endif
Hello Srini,
doesn't it make sense to instead have a CONFIG_NVMEM_SYSFS,
maked as default y, that way the default behavior will be the same,
and people not wanting it can explicitly disable it.
This also aligns with how it's done in other drivers:
$ grep SYSFS .config
# CONFIG_SYSFS_DEPRECATED is not set
CONFIG_SYSFS_SYSCALL=y
# CONFIG_DMI_SYSFS is not set
# CONFIG_FW_CFG_SYSFS is not set
# CONFIG_ISCSI_BOOT_SYSFS is not set
# CONFIG_GPIO_SYSFS is not set
# CONFIG_WATCHDOG_SYSFS is not set
CONFIG_EDAC_LEGACY_SYSFS=y
CONFIG_RTC_INTF_SYSFS=y
CONFIG_CROS_EC_SYSFS=m
# CONFIG_IIO_SYSFS_TRIGGER is not set
CONFIG_PWM_SYSFS=y
CONFIG_SYSFS=y
Kind regards,
Niklas
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index b9a0270883a0..c70f183fe379 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -216,7 +216,7 @@ static const struct attribute_group nvmem_bin_rw_group =
> {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +static const __maybe_unused struct attribute_group *nvmem_rw_dev_groups[] =
> {
> &nvmem_bin_rw_group,
> NULL,
> };
> @@ -240,7 +240,7 @@ static const struct attribute_group nvmem_bin_ro_group =
> {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +static const __maybe_unused struct attribute_group *nvmem_ro_dev_groups[] =
> {
> &nvmem_bin_ro_group,
> NULL,
> };
> @@ -265,7 +265,7 @@ static const struct attribute_group
> nvmem_bin_rw_root_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_root_dev_groups[] = {
> &nvmem_bin_rw_root_group,
> NULL,
> };
> @@ -289,7 +289,7 @@ static const struct attribute_group
> nvmem_bin_ro_root_group = {
> .attrs = nvmem_attrs,
> };
>
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_root_dev_groups[] = {
> &nvmem_bin_ro_root_group,
> NULL,
> };
> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
> nvmem->read_only = device_property_present(config->dev, "read-only") |
> config->read_only;
>
> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
> if (config->root_only)
> nvmem->dev.groups = nvmem->read_only ?
> nvmem_ro_root_dev_groups :
> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
> nvmem->dev.groups = nvmem->read_only ?
> nvmem_ro_dev_groups :
> nvmem_rw_dev_groups;
> -
> +#endif
> device_initialize(&nvmem->dev);
>
> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
> --
> 2.21.0
>
> --------------------------->cut<-----------------------------------