* [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write @ 2019-03-17 14:12 Gaurav Kohli 2019-03-20 14:34 ` Srinivas Kandagatla 0 siblings, 1 reply; 11+ messages in thread From: Gaurav Kohli @ 2019-03-17 14:12 UTC (permalink / raw) To: srinivas.kandagatla, linux-kernel; +Cc: linux-arm-msm, Gaurav Kohli 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 <gkohli@codeaurora.org> --- 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. ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-17 14:12 [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write Gaurav Kohli @ 2019-03-20 14:34 ` Srinivas Kandagatla 2019-03-20 15:50 ` Gaurav Kohli 0 siblings, 1 reply; 11+ messages in thread From: Srinivas Kandagatla @ 2019-03-20 14:34 UTC (permalink / raw) To: Gaurav Kohli, linux-kernel; +Cc: linux-arm-msm 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-20 14:34 ` Srinivas Kandagatla @ 2019-03-20 15:50 ` Gaurav Kohli 2019-03-20 16:26 ` Srinivas Kandagatla 0 siblings, 1 reply; 11+ messages in thread From: Gaurav Kohli @ 2019-03-20 15:50 UTC (permalink / raw) To: Srinivas Kandagatla, linux-kernel; +Cc: linux-arm-msm 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-20 15:50 ` Gaurav Kohli @ 2019-03-20 16:26 ` Srinivas Kandagatla 2019-03-20 17:50 ` Gaurav Kohli 0 siblings, 1 reply; 11+ messages in thread From: Srinivas Kandagatla @ 2019-03-20 16:26 UTC (permalink / raw) To: Gaurav Kohli, linux-kernel; +Cc: linux-arm-msm 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-20 16:26 ` Srinivas Kandagatla @ 2019-03-20 17:50 ` Gaurav Kohli 2019-03-21 13:14 ` Marc Gonzalez 2019-03-22 15:02 ` Srinivas Kandagatla 0 siblings, 2 replies; 11+ messages in thread From: Gaurav Kohli @ 2019-03-20 17:50 UTC (permalink / raw) To: Srinivas Kandagatla, linux-kernel; +Cc: linux-arm-msm 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-20 17:50 ` Gaurav Kohli @ 2019-03-21 13:14 ` Marc Gonzalez 2019-03-22 15:02 ` Srinivas Kandagatla 1 sibling, 0 replies; 11+ messages in thread From: Marc Gonzalez @ 2019-03-21 13:14 UTC (permalink / raw) To: Gaurav Kohli, LKML; +Cc: Srinivas Kandagatla, MSM, Arnd Bergmann, Rob Herring 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-20 17:50 ` Gaurav Kohli 2019-03-21 13:14 ` Marc Gonzalez @ 2019-03-22 15:02 ` Srinivas Kandagatla 2019-03-22 18:12 ` Gaurav Kohli 2019-04-02 7:35 ` Niklas Cassel 1 sibling, 2 replies; 11+ messages in thread From: Srinivas Kandagatla @ 2019-03-22 15:02 UTC (permalink / raw) To: Gaurav Kohli, linux-kernel; +Cc: linux-arm-msm 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 <srinivas.kandagatla@linaro.org> 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 <srinivas.kandagatla@linaro.org> --- 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<----------------------------------- ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-22 15:02 ` Srinivas Kandagatla @ 2019-03-22 18:12 ` Gaurav Kohli 2019-03-25 6:15 ` Gaurav Kohli 2019-04-02 7:35 ` Niklas Cassel 1 sibling, 1 reply; 11+ messages in thread From: Gaurav Kohli @ 2019-03-22 18:12 UTC (permalink / raw) To: Srinivas Kandagatla, linux-kernel; +Cc: linux-arm-msm 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 <srinivas.kandagatla@linaro.org> > 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 <srinivas.kandagatla@linaro.org> > --- > 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-22 18:12 ` Gaurav Kohli @ 2019-03-25 6:15 ` Gaurav Kohli 2019-04-01 4:52 ` Gaurav Kohli 0 siblings, 1 reply; 11+ messages in thread From: Gaurav Kohli @ 2019-03-25 6:15 UTC (permalink / raw) To: Srinivas Kandagatla, linux-kernel; +Cc: linux-arm-msm 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 <gkohli@codeaurora.org> Tested-by: Gaurav Kohli <gkohli@codeaurora.org> 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 <srinivas.kandagatla@linaro.org> >> 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 <srinivas.kandagatla@linaro.org> >> --- >> 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-25 6:15 ` Gaurav Kohli @ 2019-04-01 4:52 ` Gaurav Kohli 0 siblings, 0 replies; 11+ messages in thread From: Gaurav Kohli @ 2019-04-01 4:52 UTC (permalink / raw) To: Srinivas Kandagatla, linux-kernel; +Cc: linux-arm-msm 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 <gkohli@codeaurora.org> > Tested-by: Gaurav Kohli <gkohli@codeaurora.org> > > 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 <srinivas.kandagatla@linaro.org> >>> 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 <srinivas.kandagatla@linaro.org> >>> --- >>> 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write 2019-03-22 15:02 ` Srinivas Kandagatla 2019-03-22 18:12 ` Gaurav Kohli @ 2019-04-02 7:35 ` Niklas Cassel 1 sibling, 0 replies; 11+ messages in thread From: Niklas Cassel @ 2019-04-02 7:35 UTC (permalink / raw) To: Srinivas Kandagatla; +Cc: Gaurav Kohli, linux-kernel, linux-arm-msm 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 <srinivas.kandagatla@linaro.org> > 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 <srinivas.kandagatla@linaro.org> > --- > 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<----------------------------------- ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-02 7:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-17 14:12 [PATCH v2] nvmem: core: Set no-read-write provider to avoid userspace read/write Gaurav Kohli 2019-03-20 14:34 ` Srinivas Kandagatla 2019-03-20 15:50 ` Gaurav Kohli 2019-03-20 16:26 ` Srinivas Kandagatla 2019-03-20 17:50 ` Gaurav Kohli 2019-03-21 13:14 ` Marc Gonzalez 2019-03-22 15:02 ` Srinivas Kandagatla 2019-03-22 18:12 ` Gaurav Kohli 2019-03-25 6:15 ` Gaurav Kohli 2019-04-01 4:52 ` Gaurav Kohli 2019-04-02 7:35 ` Niklas Cassel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.