From: sudeep.holla@arm.com (Sudeep Holla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] drivers/base: cacheinfo: validate device node for all the caches
Date: Mon, 23 Feb 2015 15:45:06 +0000 [thread overview]
Message-ID: <54EB4B02.9070304@arm.com> (raw)
In-Reply-To: <20150223151454.GL9714@leverpostej>
On 23/02/15 15:14, Mark Rutland wrote:
> On Mon, Feb 16, 2015 at 02:10:16PM +0000, Sudeep Holla wrote:
>> On architectures that depend on DT for obtaining cache hierarcy, we need
>> to validate the device node for all the cache indices, failing to do so
>> might result in wrong information being exposed to the userspace.
>>
>> This is quite possible on initial/incomplete versions of the device
>> trees. In such cases, it's better to bail out if all the required device
>> nodes are not present.
>>
>> This patch adds checks for the validation of device node for all the
>> caches and doesn't initialise the cacheinfo if there's any error.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Reported-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/base/cacheinfo.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>> index 6e64563361f0..7015bf05c828 100644
>> --- a/drivers/base/cacheinfo.c
>> +++ b/drivers/base/cacheinfo.c
[...]
>> @@ -189,8 +195,10 @@ static int detect_cache_attributes(unsigned int cpu)
>> * will be set up here only if they are not populated already
>> */
>> ret = cache_shared_cpu_map_setup(cpu);
>> - if (ret)
>> + if (ret) {
>> + pr_err("failed to setup cache hierarcy from DT\n");
>
> It would probably be better if this were something like:
>
> pr_warn("Unable to detect cache hierarcy from DT for CPU %d\n",
> cpu);
>
Agreed, will update and send v2.
> Otherwise, this looks sane to me, and it would be nice to have this in
> ASAP so as to avoid exposing erroneous information to userspace. So:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks.
Regards,
Sudeep
WARNING: multiple messages have this Message-ID (diff)
From: Sudeep Holla <sudeep.holla@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] drivers/base: cacheinfo: validate device node for all the caches
Date: Mon, 23 Feb 2015 15:45:06 +0000 [thread overview]
Message-ID: <54EB4B02.9070304@arm.com> (raw)
In-Reply-To: <20150223151454.GL9714@leverpostej>
On 23/02/15 15:14, Mark Rutland wrote:
> On Mon, Feb 16, 2015 at 02:10:16PM +0000, Sudeep Holla wrote:
>> On architectures that depend on DT for obtaining cache hierarcy, we need
>> to validate the device node for all the cache indices, failing to do so
>> might result in wrong information being exposed to the userspace.
>>
>> This is quite possible on initial/incomplete versions of the device
>> trees. In such cases, it's better to bail out if all the required device
>> nodes are not present.
>>
>> This patch adds checks for the validation of device node for all the
>> caches and doesn't initialise the cacheinfo if there's any error.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Reported-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/base/cacheinfo.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>> index 6e64563361f0..7015bf05c828 100644
>> --- a/drivers/base/cacheinfo.c
>> +++ b/drivers/base/cacheinfo.c
[...]
>> @@ -189,8 +195,10 @@ static int detect_cache_attributes(unsigned int cpu)
>> * will be set up here only if they are not populated already
>> */
>> ret = cache_shared_cpu_map_setup(cpu);
>> - if (ret)
>> + if (ret) {
>> + pr_err("failed to setup cache hierarcy from DT\n");
>
> It would probably be better if this were something like:
>
> pr_warn("Unable to detect cache hierarcy from DT for CPU %d\n",
> cpu);
>
Agreed, will update and send v2.
> Otherwise, this looks sane to me, and it would be nice to have this in
> ASAP so as to avoid exposing erroneous information to userspace. So:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks.
Regards,
Sudeep
next prev parent reply other threads:[~2015-02-23 15:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-16 14:10 [PATCH] drivers/base: cacheinfo: validate device node for all the caches Sudeep Holla
2015-02-16 14:10 ` Sudeep Holla
2015-02-23 15:14 ` Mark Rutland
2015-02-23 15:14 ` Mark Rutland
2015-02-23 15:45 ` Sudeep Holla [this message]
2015-02-23 15:45 ` Sudeep Holla
2015-02-23 16:32 ` [PATCH v2] " Sudeep Holla
2015-02-23 16:32 ` Sudeep Holla
2015-02-23 16:32 ` Sudeep Holla
2015-03-10 9:52 ` Sudeep Holla
2015-03-10 9:52 ` Sudeep Holla
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=54EB4B02.9070304@arm.com \
--to=sudeep.holla@arm.com \
--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.