Hello all,
Previous version: https://lkml.org/lkml/2020/2/24/973
Changed since previous version:
- No longer looking to drop read_only flag in this series because of
reasons given by Srinivas. Fixed some areas where I was ignoring the
read_only flag and relying solely on presence of reg_read to determine
if read only.
- Changed nvmem_register() to return failure if group is NULL from
nvmem_sysfs_get_groups().
- Added commit to check for NULL reg_read and reg_write before
dereferencing
- No longer using WARN_ON() because now we can return NULL from
nvmem_sysfs_get_groups() if the inputs are unacceptable. Much nicer.
- No longer providing global writable entry - my bad.
Not changed since previous version despite discussion:
- Not changing read_only flag to world_writable and inverting logic - it
would mean that all of the drivers that do not set the flag now need
to set it and vice-versa. Too much verification for now. I might come
back to these things later, but for now, I realised my eyes should be
on the main goal.
- Not documented kernel-doc of struct nvmem_config. Unclear if this was
required because of WARN_ON() - if so, this no longer applies, as
WARN_ON() was removed. If this is still an issue, please clarify whether
this means Documentation/driver-api/nvmem.rst or just the blurb in
include/linux/nvmem-provider.h, and what specific change needs
documenting.
Nicholas Johnson (3):
nvmem: Add support for write-only instances
nvmem: check for NULL reg_read and reg_write before dereferencing
Revert "thunderbolt: Prevent crash if non-active NVMem file is read"
drivers/nvmem/core.c | 2 ++
drivers/nvmem/nvmem-sysfs.c | 59 +++++++++++++++++++++++++++++++-----
drivers/thunderbolt/switch.c | 7 -----
3 files changed, 54 insertions(+), 14 deletions(-)
--
2.25.1
This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
Since NVMEM subsystem gained support for write-only instances, this
workaround is no longer required, so drop it.
Signed-off-by: Nicholas Johnson <[email protected]>
---
drivers/thunderbolt/switch.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 7d6ecc342..ad5479f21 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -348,12 +348,6 @@ static int tb_switch_nvm_read(void *priv, unsigned int offset, void *val,
return ret;
}
-static int tb_switch_nvm_no_read(void *priv, unsigned int offset, void *val,
- size_t bytes)
-{
- return -EPERM;
-}
-
static int tb_switch_nvm_write(void *priv, unsigned int offset, void *val,
size_t bytes)
{
@@ -399,7 +393,6 @@ static struct nvmem_device *register_nvmem(struct tb_switch *sw, int id,
config.read_only = true;
} else {
config.name = "nvm_non_active";
- config.reg_read = tb_switch_nvm_no_read;
config.reg_write = tb_switch_nvm_write;
config.root_only = true;
}
--
2.25.1
There is at least one real-world use-case for write-only nvmem
instances. Refer to 03cd45d2e219 ("thunderbolt: Prevent crash if
non-active NVMem file is read").
Add support for write-only nvmem instances by adding attrs for 0200.
Change nvmem_register() to abort if NULL group is returned from
nvmem_sysfs_get_groups().
Return NULL from nvmem_sysfs_get_groups() in invalid cases.
Signed-off-by: Nicholas Johnson <[email protected]>
---
drivers/nvmem/core.c | 2 ++
drivers/nvmem/nvmem-sysfs.c | 53 ++++++++++++++++++++++++++++++++-----
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ef326f243..27bd4c4e3 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -388,6 +388,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
config->read_only || !nvmem->reg_write;
nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
+ if (!nvmem->dev.groups)
+ return NULL;
device_initialize(&nvmem->dev);
diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
index 9e0c429cd..00d3259ea 100644
--- a/drivers/nvmem/nvmem-sysfs.c
+++ b/drivers/nvmem/nvmem-sysfs.c
@@ -196,16 +196,50 @@ static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
NULL,
};
+/* write only permission, root only */
+static struct bin_attribute bin_attr_wo_root_nvmem = {
+ .attr = {
+ .name = "nvmem",
+ .mode = 0200,
+ },
+ .write = bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_wo_root_attributes[] = {
+ &bin_attr_wo_root_nvmem,
+ NULL,
+};
+
+static const struct attribute_group nvmem_bin_wo_root_group = {
+ .bin_attrs = nvmem_bin_wo_root_attributes,
+ .attrs = nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_wo_root_dev_groups[] = {
+ &nvmem_bin_wo_root_group,
+ NULL,
+};
+
const struct attribute_group **nvmem_sysfs_get_groups(
struct nvmem_device *nvmem,
const struct nvmem_config *config)
{
- if (config->root_only)
- return nvmem->read_only ?
- nvmem_ro_root_dev_groups :
- nvmem_rw_root_dev_groups;
-
- return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
+ /* Read-only */
+ if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only))
+ return config->root_only ?
+ nvmem_ro_root_dev_groups : nvmem_ro_dev_groups;
+
+ /* Read-write */
+ if (nvmem->reg_read && nvmem->reg_write)
+ return config->root_only ?
+ nvmem_rw_root_dev_groups : nvmem_rw_dev_groups;
+
+ /* Write-only, do not honour request for global writable entry */
+ if (!nvmem->reg_read && nvmem->reg_write)
+ return config->root_only ? nvmem_wo_root_dev_groups : NULL;
+
+ /* Neither reg_read nor reg_write are provided, abort */
+ return NULL;
}
/*
@@ -224,11 +258,16 @@ int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
if (!config->base_dev)
return -EINVAL;
- if (nvmem->read_only) {
+ if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only)) {
if (config->root_only)
nvmem->eeprom = bin_attr_ro_root_nvmem;
else
nvmem->eeprom = bin_attr_ro_nvmem;
+ } else if (!nvmem->reg_read && nvmem->reg_write) {
+ if (config->root_only)
+ nvmem->eeprom = bin_attr_wo_root_nvmem;
+ else
+ return -EPERM;
} else {
if (config->root_only)
nvmem->eeprom = bin_attr_rw_root_nvmem;
--
2.25.1
Return -EPERM if reg_read is NULL in bin_attr_nvmem_read() or if
reg_write is NULL in bin_attr_nvmem_write().
This prevents NULL dereferences such as the one described in
03cd45d2e219 ("thunderbolt: Prevent crash if non-active NVMem file is
read")
Signed-off-by: Nicholas Johnson <[email protected]>
---
drivers/nvmem/nvmem-sysfs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
index 00d3259ea..9312e1d6f 100644
--- a/drivers/nvmem/nvmem-sysfs.c
+++ b/drivers/nvmem/nvmem-sysfs.c
@@ -56,6 +56,9 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
count = round_down(count, nvmem->word_size);
+ if (!nvmem->reg_read)
+ return -EPERM;
+
rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
if (rc)
@@ -90,6 +93,9 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
count = round_down(count, nvmem->word_size);
+ if (!nvmem->reg_write)
+ return -EPERM;
+
rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
if (rc)
--
2.25.1
On Mon, Mar 02, 2020 at 03:43:02PM +0000, Nicholas Johnson wrote:
> Return -EPERM if reg_read is NULL in bin_attr_nvmem_read() or if
> reg_write is NULL in bin_attr_nvmem_write().
Hmm, is this patch required at all since you already check the invalid
combinations in the patch 1/3?
On Mon, Mar 02, 2020 at 03:43:29PM +0000, Nicholas Johnson wrote:
> This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
>
> Since NVMEM subsystem gained support for write-only instances, this
> workaround is no longer required, so drop it.
>
> Signed-off-by: Nicholas Johnson <[email protected]>
Assuming this goes through The NVMem tree:
Acked-by: Mika Westerberg <[email protected]>
If that's not the case, please let me know. I can also take them through
the Thunderbolt tree.
On Tue, Mar 03, 2020 at 12:29:56PM +0200, Mika Westerberg wrote:
> On Mon, Mar 02, 2020 at 03:43:02PM +0000, Nicholas Johnson wrote:
> > Return -EPERM if reg_read is NULL in bin_attr_nvmem_read() or if
> > reg_write is NULL in bin_attr_nvmem_write().
>
> Hmm, is this patch required at all since you already check the invalid
> combinations in the patch 1/3?
This is checked in nvmem_reg_read() and nvmem_reg_write() in
drivers/nvmem/core.c - for consistency, perhaps both should be done.
Also, defensive programming is a good idea, because code changes might
allow for a NULL to slip through in the future. But you are correct, at
this moment in time, we should not be able to cause a NULL dereference
(at least as far as I can tell).
Srinivas can either apply this patch or not, so if it is decided this is
not needed, I will not need to do a PATCH v3. Patch 3/3 does not
conflict with this patch.
Cheers
On Tue, Mar 03, 2020 at 12:33:10PM +0200, Mika Westerberg wrote:
> On Mon, Mar 02, 2020 at 03:43:29PM +0000, Nicholas Johnson wrote:
> > This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
> >
> > Since NVMEM subsystem gained support for write-only instances, this
> > workaround is no longer required, so drop it.
> >
> > Signed-off-by: Nicholas Johnson <[email protected]>
>
> Assuming this goes through The NVMem tree:
>
> Acked-by: Mika Westerberg <[email protected]>
>
> If that's not the case, please let me know. I can also take them through
> the Thunderbolt tree.
I do not know how this would normally work - I have not experienced much
cross-subsystem work. Perhaps it should be taken through your tree. If
it goes through your tree and not part of this series, perhaps it does
not make sense for it to be authored by me, either. It's just a revert;
it does not take a lot of effort or doing something original.
Cheers.
On Wed, Mar 04, 2020 at 04:07:29PM +0000, Nicholas Johnson wrote:
> On Tue, Mar 03, 2020 at 12:33:10PM +0200, Mika Westerberg wrote:
> > On Mon, Mar 02, 2020 at 03:43:29PM +0000, Nicholas Johnson wrote:
> > > This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
> > >
> > > Since NVMEM subsystem gained support for write-only instances, this
> > > workaround is no longer required, so drop it.
> > >
> > > Signed-off-by: Nicholas Johnson <[email protected]>
> >
> > Assuming this goes through The NVMem tree:
> >
> > Acked-by: Mika Westerberg <[email protected]>
> >
> > If that's not the case, please let me know. I can also take them through
> > the Thunderbolt tree.
> I do not know how this would normally work - I have not experienced much
> cross-subsystem work. Perhaps it should be taken through your tree. If
> it goes through your tree and not part of this series, perhaps it does
> not make sense for it to be authored by me, either. It's just a revert;
> it does not take a lot of effort or doing something original.
Your authorship is fine.
Since this patch depends on the first one, it should go together with
that one either to NVMem tree or Thunderbolt tree. Either is fine by me
but if I take them then I need an ack from Srinivas.
On 02/03/2020 15:42, Nicholas Johnson wrote:
> There is at least one real-world use-case for write-only nvmem
> instances. Refer to 03cd45d2e219 ("thunderbolt: Prevent crash if
> non-active NVMem file is read").
>
> Add support for write-only nvmem instances by adding attrs for 0200.
>
> Change nvmem_register() to abort if NULL group is returned from
> nvmem_sysfs_get_groups().
>
> Return NULL from nvmem_sysfs_get_groups() in invalid cases.
>
> Signed-off-by: Nicholas Johnson <[email protected]>
> ---
> drivers/nvmem/core.c | 2 ++
> drivers/nvmem/nvmem-sysfs.c | 53 ++++++++++++++++++++++++++++++++-----
> 2 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index ef326f243..27bd4c4e3 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -388,6 +388,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> config->read_only || !nvmem->reg_write;
>
> nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
> + if (!nvmem->dev.groups)
> + return NULL;
Returning here will be leaking in this function.
>
> device_initialize(&nvmem->dev);
>
> diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
> index 9e0c429cd..00d3259ea 100644
> --- a/drivers/nvmem/nvmem-sysfs.c
> +++ b/drivers/nvmem/nvmem-sysfs.c
> @@ -196,16 +196,50 @@ static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> NULL,
> };
>
> +/* write only permission, root only */
> +static struct bin_attribute bin_attr_wo_root_nvmem = {
TBH, you would not need this patch once 2/3 patch is applied.
Unless there is a strong reason for you to have write only file.
If for any reasons you would want to add Write only file then it should
be added for both with root and user privileges.
> const struct attribute_group **nvmem_sysfs_get_groups(
> struct nvmem_device *nvmem,
> const struct nvmem_config *config)
> {
> - if (config->root_only)
> - return nvmem->read_only ?
> - nvmem_ro_root_dev_groups :
> - nvmem_rw_root_dev_groups;
> -
> - return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
> + /* Read-only */
> + if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only))
> + return config->root_only ?
> + nvmem_ro_root_dev_groups : nvmem_ro_dev_groups;
> +
> + /* Read-write */
> + if (nvmem->reg_read && nvmem->reg_write)
read_only flag will override this assumption!
> + return config->root_only ?
> + nvmem_rw_root_dev_groups : nvmem_rw_dev_groups;
> +
> + /* Write-only, do not honour request for global writable entry */
> + if (!nvmem->reg_read && nvmem->reg_write)
> + return config->root_only ? nvmem_wo_root_dev_groups : NULL;
> +
> + /* Neither reg_read nor reg_write are provided, abort */
This should not be the case anymore after this check in place
https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/commit/?h=for-next&id=f8f782f63bace8b08362e466747e648ca57b6c06
thanks,
srini
> + return NULL;
> }
>
> /*
> @@ -224,11 +258,16 @@ int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> if (!config->base_dev)
> return -EINVAL;
>
> - if (nvmem->read_only) {
> + if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only)) {
> if (config->root_only)
> nvmem->eeprom = bin_attr_ro_root_nvmem;
> else
> nvmem->eeprom = bin_attr_ro_nvmem;
> + } else if (!nvmem->reg_read && nvmem->reg_write) {
> + if (config->root_only)
> + nvmem->eeprom = bin_attr_wo_root_nvmem;
> + else
> + return -EPERM;
> } else {
> if (config->root_only)
> nvmem->eeprom = bin_attr_rw_root_nvmem;
>
On 02/03/2020 15:43, Nicholas Johnson wrote:
> Return -EPERM if reg_read is NULL in bin_attr_nvmem_read() or if
> reg_write is NULL in bin_attr_nvmem_write().
>
> This prevents NULL dereferences such as the one described in
> 03cd45d2e219 ("thunderbolt: Prevent crash if non-active NVMem file is
> read")
>
> Signed-off-by: Nicholas Johnson <[email protected]>
> ---
> drivers/nvmem/nvmem-sysfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
Applied thanks,
--srini
On 04/03/2020 16:18, Mika Westerberg wrote:
> On Wed, Mar 04, 2020 at 04:07:29PM +0000, Nicholas Johnson wrote:
>> On Tue, Mar 03, 2020 at 12:33:10PM +0200, Mika Westerberg wrote:
>>> On Mon, Mar 02, 2020 at 03:43:29PM +0000, Nicholas Johnson wrote:
>>>> This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
>>>>
>>>> Since NVMEM subsystem gained support for write-only instances, this
>>>> workaround is no longer required, so drop it.
>>>>
>>>> Signed-off-by: Nicholas Johnson <[email protected]>
>>>
>>> Assuming this goes through The NVMem tree:
>>>
>>> Acked-by: Mika Westerberg <[email protected]>
>>>
>>> If that's not the case, please let me know. I can also take them through
>>> the Thunderbolt tree.
>> I do not know how this would normally work - I have not experienced much
>> cross-subsystem work. Perhaps it should be taken through your tree. If
>> it goes through your tree and not part of this series, perhaps it does
>> not make sense for it to be authored by me, either. It's just a revert;
>> it does not take a lot of effort or doing something original.
>
> Your authorship is fine.
>
> Since this patch depends on the first one, it should go together with
> that one either to NVMem tree or Thunderbolt tree. Either is fine by me
> but if I take them then I need an ack from Srinivas.
>
I applied 2/3 patch which should show up in next 5.7-rc1 release, with
that in place you can revert this patch. Please take this patch via
respective tree, it does not make much sense for me to apply this as its
not going to break any build.
--srini
On Thu, Mar 05, 2020 at 05:03:11PM +0000, Srinivas Kandagatla wrote:
>
> On 02/03/2020 15:42, Nicholas Johnson wrote:
> > There is at least one real-world use-case for write-only nvmem
> > instances. Refer to 03cd45d2e219 ("thunderbolt: Prevent crash if
> > non-active NVMem file is read").
> >
> > Add support for write-only nvmem instances by adding attrs for 0200.
> >
> > Change nvmem_register() to abort if NULL group is returned from
> > nvmem_sysfs_get_groups().
> >
> > Return NULL from nvmem_sysfs_get_groups() in invalid cases.
> >
>
> > Signed-off-by: Nicholas Johnson <[email protected]>
> > ---
> > drivers/nvmem/core.c | 2 ++
> > drivers/nvmem/nvmem-sysfs.c | 53 ++++++++++++++++++++++++++++++++-----
> > 2 files changed, 48 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index ef326f243..27bd4c4e3 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -388,6 +388,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> > config->read_only || !nvmem->reg_write;
> > nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
> > + if (!nvmem->dev.groups)
> > + return NULL;
> Returning here will be leaking in this function.
Oops, I had thought about that but missed it. Will kfree(nvmem) in next
revision of this patch. I will probably break this change off into
another commit to make each commit smaller and do one thing.
>
> > device_initialize(&nvmem->dev);
> > diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
> > index 9e0c429cd..00d3259ea 100644
> > --- a/drivers/nvmem/nvmem-sysfs.c
> > +++ b/drivers/nvmem/nvmem-sysfs.c
> > @@ -196,16 +196,50 @@ static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> > NULL,
> > };
> > +/* write only permission, root only */
> > +static struct bin_attribute bin_attr_wo_root_nvmem = {
>
> TBH, you would not need this patch once 2/3 patch is applied.
> Unless there is a strong reason for you to have write only file.
This was the whole reason. The Thunderbolt NULL dereference was because
write-only was needed but not available. Mika Westerberg thought that by
not providing reg_read, the nvmem would become write-only. I discovered
the NULL dereference and that is why I am here - to provide the
sought-after write-only support. So yes, there is a reason to have
write-only.
>
> If for any reasons you would want to add Write only file then it should be
> added for both with root and user privileges.
Mika just advised me that we should not have world-writable files, so it
sounds like this needs some discussion between us. I am happy to provide
this if that is desired, as the world-writable will presumably only be
used if there is a driver that asks for it and has a good reason to use
it, so it should not be unsafe.
Part of me agrees that there should be no need for world-writable (I
cannot think of a use-case) but the other part knows that something
could come along, and that we should cover all bases. Just like we did
not see the need for write-only.
>
> > const struct attribute_group **nvmem_sysfs_get_groups(
> > struct nvmem_device *nvmem,
> > const struct nvmem_config *config)
> > {
> > - if (config->root_only)
> > - return nvmem->read_only ?
> > - nvmem_ro_root_dev_groups :
> > - nvmem_rw_root_dev_groups;
> > -
> > - return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
> > + /* Read-only */
> > + if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only))
> > + return config->root_only ?
> > + nvmem_ro_root_dev_groups : nvmem_ro_dev_groups;
> > +
> > + /* Read-write */
> > + if (nvmem->reg_read && nvmem->reg_write)
>
> read_only flag will override this assumption!
If reg_read != NULL and read_only set then we have already returned.
Setting read_only and having reg_read == NULL is clearly broken
behaviour. How about I explicitly check for reg_read == NULL and
read_only set, and return NULL?
>
> > + return config->root_only ?
> > + nvmem_rw_root_dev_groups : nvmem_rw_dev_groups;
> > +
> > + /* Write-only, do not honour request for global writable entry */
> > + if (!nvmem->reg_read && nvmem->reg_write)
> > + return config->root_only ? nvmem_wo_root_dev_groups : NULL;
> > +
> > + /* Neither reg_read nor reg_write are provided, abort */
> This should not be the case anymore after this check in place
>
> https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/commit/?h=for-next&id=f8f782f63bace8b08362e466747e648ca57b6c06
Nice. I can change the comment, but all code paths need a return value.
I do not know if the compiler is smart enough to figure out that the
final return statement is unreachable. So I will still be returning NULL
at the end to avoid warnings.
>
> thanks,
> srini
Thanks for reviewing.
Kind regards,
Nicholas
>
> > + return NULL;
> > }
> > /*
> > @@ -224,11 +258,16 @@ int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> > if (!config->base_dev)
> > return -EINVAL;
> > - if (nvmem->read_only) {
> > + if (nvmem->reg_read && (!nvmem->reg_write || nvmem->read_only)) {
> > if (config->root_only)
> > nvmem->eeprom = bin_attr_ro_root_nvmem;
> > else
> > nvmem->eeprom = bin_attr_ro_nvmem;
> > + } else if (!nvmem->reg_read && nvmem->reg_write) {
> > + if (config->root_only)
> > + nvmem->eeprom = bin_attr_wo_root_nvmem;
> > + else
> > + return -EPERM;
> > } else {
> > if (config->root_only)
> > nvmem->eeprom = bin_attr_rw_root_nvmem;
> >
On 05/03/2020 18:11, Nicholas Johnson wrote:
>> If for any reasons you would want to add Write only file then it should be
>> added for both with root and user privileges.
> Mika just advised me that we should not have world-writable files
I totally agree with him, should have been careful before replying it
too early!
lets keep root only writeable file.
--srini
On Thu, Mar 05, 2020 at 05:05:47PM +0000, Srinivas Kandagatla wrote:
>
>
> On 04/03/2020 16:18, Mika Westerberg wrote:
> > On Wed, Mar 04, 2020 at 04:07:29PM +0000, Nicholas Johnson wrote:
> > > On Tue, Mar 03, 2020 at 12:33:10PM +0200, Mika Westerberg wrote:
> > > > On Mon, Mar 02, 2020 at 03:43:29PM +0000, Nicholas Johnson wrote:
> > > > > This reverts commit 03cd45d2e219301880cabc357e3cf478a500080f.
> > > > >
> > > > > Since NVMEM subsystem gained support for write-only instances, this
> > > > > workaround is no longer required, so drop it.
> > > > >
> > > > > Signed-off-by: Nicholas Johnson <[email protected]>
> > > >
> > > > Assuming this goes through The NVMem tree:
> > > >
> > > > Acked-by: Mika Westerberg <[email protected]>
> > > >
> > > > If that's not the case, please let me know. I can also take them through
> > > > the Thunderbolt tree.
> > > I do not know how this would normally work - I have not experienced much
> > > cross-subsystem work. Perhaps it should be taken through your tree. If
> > > it goes through your tree and not part of this series, perhaps it does
> > > not make sense for it to be authored by me, either. It's just a revert;
> > > it does not take a lot of effort or doing something original.
> >
> > Your authorship is fine.
> >
> > Since this patch depends on the first one, it should go together with
> > that one either to NVMem tree or Thunderbolt tree. Either is fine by me
> > but if I take them then I need an ack from Srinivas.
> >
>
> I applied 2/3 patch which should show up in next 5.7-rc1 release, with that
> in place you can revert this patch. Please take this patch via respective
> tree, it does not make much sense for me to apply this as its not going to
> break any build.
OK, that works for me as well.
Nicholas, can you send this one again after 5.7-rc1 is is released? I
can then pick it up to my tree.