All of lore.kernel.org
 help / color / mirror / Atom feed
From: srinivas.kandagatla@linaro.org (Srinivas Kandagatla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] nvmem: core: make default user binary file root-access only
Date: Wed, 07 Oct 2015 18:26:02 +0100	[thread overview]
Message-ID: <561555AA.5090000@linaro.org> (raw)
In-Reply-To: <20151007171227.GA9485@kroah.com>



On 07/10/15 18:12, Greg KH wrote:
> On Wed, Oct 07, 2015 at 06:01:03PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 07/10/15 17:50, Greg KH wrote:
>>> On Wed, Oct 07, 2015 at 05:35:14PM +0100, Srinivas Kandagatla wrote:
>>>> As required by many providers like at24/at25/mxs-ocotp/qfprom, which would
>>>> want to allow root-only to read/write the nvmem content.
>>>> So making the defaults to be root-only access which can prevent normal
>>>> users from reading the nvmem data.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>> ---
>>>>   drivers/nvmem/core.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>> index 6fd4e5a..4d2e476 100644
>>>> --- a/drivers/nvmem/core.c
>>>> +++ b/drivers/nvmem/core.c
>>>> @@ -112,7 +112,7 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
>>>>   static struct bin_attribute bin_attr_rw_nvmem = {
>>>>   	.attr	= {
>>>>   		.name	= "nvmem",
>>>> -		.mode	= S_IWUSR | S_IRUGO,
>>>> +		.mode	= S_IWUSR | S_IRUSR,
>>>>   	},
>>>>   	.read	= bin_attr_nvmem_read,
>>>>   	.write	= bin_attr_nvmem_write,
>>>> @@ -136,7 +136,7 @@ static const struct attribute_group *nvmem_rw_dev_groups[] = {
>>>>   static struct bin_attribute bin_attr_ro_nvmem = {
>>>>   	.attr	= {
>>>>   		.name	= "nvmem",
>>>> -		.mode	= S_IRUGO,
>>>> +		.mode	= S_IRUSR,
>>>>   	},
>>>>   	.read	= bin_attr_nvmem_read,
>>>>   };
>>>
>>> How about using BIN_ATTR_RO() and friends instead, that way I _know_ you
>>> got the permissions correct as it's impossible to get them wrong by
>>> using those macros.
>> Yes, that sounds good, but there are no macros for just "S_IRUSR" or
>> "(S_IWUSR | S_IRUSR)" in ./include/linux/sysfs.h
>
> Then that means you are trying to do something "odd" and you shouldn't
> be doing that :)
>
The requirement originally came from a discussion regarding file 
permissions set by at24 (drivers/mis/eeprom/at24.c) vs nvmem.

at24 driver sets the permission of root-only read/write, which is 
different to what nvmem sets as default.

So this patch was primarily make nvmem framework inline with what 
at24/at25 does.

Other argument was regarding the content of the nvmem which could have 
things like keys and normal user should not be allowed read it.

--srini
> Use the standard attribute permissions (RO, RW), as those are the
> "safest" and what you really want to be using here, as you already are.
>

> thanks,
>
> greg k-h
>

WARNING: multiple messages have this Message-ID (diff)
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: maxime.ripard@free-electrons.com, linux-kernel@vger.kernel.org,
	andrew@lunn.ch, wxt@rock-chips.com, stefan.wahren@i2se.com,
	pantelis.antoniou@konsulko.com, maitysanchayan@gmail.com,
	p.zabel@pengutronix.de, s.hauer@pengutronix.de,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] nvmem: core: make default user binary file root-access only
Date: Wed, 07 Oct 2015 18:26:02 +0100	[thread overview]
Message-ID: <561555AA.5090000@linaro.org> (raw)
In-Reply-To: <20151007171227.GA9485@kroah.com>



On 07/10/15 18:12, Greg KH wrote:
> On Wed, Oct 07, 2015 at 06:01:03PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 07/10/15 17:50, Greg KH wrote:
>>> On Wed, Oct 07, 2015 at 05:35:14PM +0100, Srinivas Kandagatla wrote:
>>>> As required by many providers like at24/at25/mxs-ocotp/qfprom, which would
>>>> want to allow root-only to read/write the nvmem content.
>>>> So making the defaults to be root-only access which can prevent normal
>>>> users from reading the nvmem data.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>> ---
>>>>   drivers/nvmem/core.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>> index 6fd4e5a..4d2e476 100644
>>>> --- a/drivers/nvmem/core.c
>>>> +++ b/drivers/nvmem/core.c
>>>> @@ -112,7 +112,7 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
>>>>   static struct bin_attribute bin_attr_rw_nvmem = {
>>>>   	.attr	= {
>>>>   		.name	= "nvmem",
>>>> -		.mode	= S_IWUSR | S_IRUGO,
>>>> +		.mode	= S_IWUSR | S_IRUSR,
>>>>   	},
>>>>   	.read	= bin_attr_nvmem_read,
>>>>   	.write	= bin_attr_nvmem_write,
>>>> @@ -136,7 +136,7 @@ static const struct attribute_group *nvmem_rw_dev_groups[] = {
>>>>   static struct bin_attribute bin_attr_ro_nvmem = {
>>>>   	.attr	= {
>>>>   		.name	= "nvmem",
>>>> -		.mode	= S_IRUGO,
>>>> +		.mode	= S_IRUSR,
>>>>   	},
>>>>   	.read	= bin_attr_nvmem_read,
>>>>   };
>>>
>>> How about using BIN_ATTR_RO() and friends instead, that way I _know_ you
>>> got the permissions correct as it's impossible to get them wrong by
>>> using those macros.
>> Yes, that sounds good, but there are no macros for just "S_IRUSR" or
>> "(S_IWUSR | S_IRUSR)" in ./include/linux/sysfs.h
>
> Then that means you are trying to do something "odd" and you shouldn't
> be doing that :)
>
The requirement originally came from a discussion regarding file 
permissions set by at24 (drivers/mis/eeprom/at24.c) vs nvmem.

at24 driver sets the permission of root-only read/write, which is 
different to what nvmem sets as default.

So this patch was primarily make nvmem framework inline with what 
at24/at25 does.

Other argument was regarding the content of the nvmem which could have 
things like keys and normal user should not be allowed read it.

--srini
> Use the standard attribute permissions (RO, RW), as those are the
> "safest" and what you really want to be using here, as you already are.
>

> thanks,
>
> greg k-h
>

  reply	other threads:[~2015-10-07 17:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-07 16:35 [PATCH] nvmem: core: make default user binary file root-access only Srinivas Kandagatla
2015-10-07 16:35 ` Srinivas Kandagatla
2015-10-07 16:50 ` Greg KH
2015-10-07 16:50   ` Greg KH
2015-10-07 17:01   ` Srinivas Kandagatla
2015-10-07 17:01     ` Srinivas Kandagatla
2015-10-07 17:12     ` Greg KH
2015-10-07 17:12       ` Greg KH
2015-10-07 17:26       ` Srinivas Kandagatla [this message]
2015-10-07 17:26         ` Srinivas Kandagatla
2015-10-07 17:38         ` Greg KH
2015-10-07 17:38           ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=561555AA.5090000@linaro.org \
    --to=srinivas.kandagatla@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.