2012-10-19 12:00:22

by Florian Fainelli

Subject: [PATCH] pstore/ram: fix undefined usage of rounddown_pow_of_two.

From: Maxime Bizon <[email protected]>

record_size / console_size / ftrace_size can be 0 (this is how you
disable the feature), but rounddown_pow_of_two(0) is undefined. This problem
has been present since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).

Signed-off-by: Maxime Bizon <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
CC: [email protected]
---
fs/pstore/ram.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 1a4f6da..0c2ae26 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -374,10 +374,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
goto fail_out;
}

- pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
- pdata->record_size = rounddown_pow_of_two(pdata->record_size);
- pdata->console_size = rounddown_pow_of_two(pdata->console_size);
- pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
+ if (pdata->mem_size)
+ pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
+ if (pdata->record_size)
+ pdata->record_size = rounddown_pow_of_two(pdata->record_size);
+ if (pdata->console_size)
+ pdata->console_size = rounddown_pow_of_two(pdata->console_size);
+ if (pdata->ftrace_size)
+ pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);

cxt->dump_read_cnt = 0;
cxt->size = pdata->mem_size;
--
1.7.9.5

2012-10-19 16:03:15

by Kees Cook

Subject: Re: [PATCH] pstore/ram: fix undefined usage of rounddown_pow_of_two.

On Fri, Oct 19, 2012 at 4:59 AM, Florian Fainelli <[email protected]> wrote:
> From: Maxime Bizon <[email protected]>
>
> record_size / console_size / ftrace_size can be 0 (this is how you
> disable the feature), but rounddown_pow_of_two(0) is undefined. This problem
> has been present since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).
>
> Signed-off-by: Maxime Bizon <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>
> CC: [email protected]
> ---
> fs/pstore/ram.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 1a4f6da..0c2ae26 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -374,10 +374,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
> goto fail_out;
> }
>
> - pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
> - pdata->record_size = rounddown_pow_of_two(pdata->record_size);
> - pdata->console_size = rounddown_pow_of_two(pdata->console_size);
> - pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
> + if (pdata->mem_size)
> + pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);

Nice catch!

Instead of the == 0 check, what about using !is_power_of_2(size) ?

-Kees

--
Kees Cook
Chrome OS Security

2012-10-22 08:38:29

by Florian Fainelli

Subject: Re: [PATCH] pstore/ram: fix undefined usage of rounddown_pow_of_two.

On Friday 19 October 2012 09:03:12 Kees Cook wrote:
> On Fri, Oct 19, 2012 at 4:59 AM, Florian Fainelli <[email protected]> wrote:
> > From: Maxime Bizon <[email protected]>
> >
> > record_size / console_size / ftrace_size can be 0 (this is how you
> > disable the feature), but rounddown_pow_of_two(0) is undefined. This problem
> > has been present since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).
> >
> > Signed-off-by: Maxime Bizon <[email protected]>
> > Signed-off-by: Florian Fainelli <[email protected]>
> > CC: [email protected]
> > ---
> > fs/pstore/ram.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> > index 1a4f6da..0c2ae26 100644
> > --- a/fs/pstore/ram.c
> > +++ b/fs/pstore/ram.c
> > @@ -374,10 +374,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
> > goto fail_out;
> > }
> >
> > - pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
> > - pdata->record_size = rounddown_pow_of_two(pdata->record_size);
> > - pdata->console_size = rounddown_pow_of_two(pdata->console_size);
> > - pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
> > + if (pdata->mem_size)
> > + pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
>
> Nice catch!
>
> Instead of the == 0 check, what about using !is_power_of_2(size) ?

That would work equally well, I will resubmit with this then.
--
Florian

2012-10-22 09:20:50

by Florian Fainelli

Subject: [PATCH v2] pstore/ram: fix undefined usage of rounddown_pow_of_two(0)

From: Maxime Bizon <[email protected]>

record_size / console_size / ftrace_size can be 0 (this is how you
disable the feature), but rounddown_pow_of_two(0) is undefined. As suggested
by Kees Cook, use !is_power_of_2() as a condition to call rounddown_pow_of_two
and avoid its undefined behavior on the value 0. This issue has been present
since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).

CC: [email protected]
Signed-off-by: Maxime Bizon <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v1:
- use !is_power_of_2(size) instead of (!size)

fs/pstore/ram.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 1a4f6da..bdd840d 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -374,10 +374,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev)
goto fail_out;
}

- pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
- pdata->record_size = rounddown_pow_of_two(pdata->record_size);
- pdata->console_size = rounddown_pow_of_two(pdata->console_size);
- pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
+ if (!is_power_of_2(pdata->mem_size))
+ pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
+ if (!is_power_of_2(pdata->record_size))
+ pdata->record_size = rounddown_pow_of_two(pdata->record_size);
+ if (!is_power_of_2(pdata->console_size))
+ pdata->console_size = rounddown_pow_of_two(pdata->console_size);
+ if (!is_power_of_2(pdata->ftrace_size))
+ pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);

cxt->dump_read_cnt = 0;
cxt->size = pdata->mem_size;
--
1.7.9.5

2012-10-22 15:17:35

by Kees Cook

Subject: Re: [PATCH v2] pstore/ram: fix undefined usage of rounddown_pow_of_two(0)

On Mon, Oct 22, 2012 at 2:19 AM, Florian Fainelli <[email protected]> wrote:
> From: Maxime Bizon <[email protected]>
>
> record_size / console_size / ftrace_size can be 0 (this is how you
> disable the feature), but rounddown_pow_of_two(0) is undefined. As suggested
> by Kees Cook, use !is_power_of_2() as a condition to call rounddown_pow_of_two
> and avoid its undefined behavior on the value 0. This issue has been present
> since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).
>
> CC: [email protected]
> Signed-off-by: Maxime Bizon <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>

Thanks for the fix!

Acked-by: Kees Cook <[email protected]>

-Kees

--
Kees Cook
Chrome OS Security

2012-11-18 01:45:39

by Anton Vorontsov

Subject: Re: [PATCH v2] pstore/ram: fix undefined usage of rounddown_pow_of_two(0)

On Mon, Oct 22, 2012 at 08:17:27AM -0700, Kees Cook wrote:
> On Mon, Oct 22, 2012 at 2:19 AM, Florian Fainelli <[email protected]> wrote:
> > From: Maxime Bizon <[email protected]>
> >
> > record_size / console_size / ftrace_size can be 0 (this is how you
> > disable the feature), but rounddown_pow_of_two(0) is undefined. As suggested
> > by Kees Cook, use !is_power_of_2() as a condition to call rounddown_pow_of_two
> > and avoid its undefined behavior on the value 0. This issue has been present
> > since commit 1894a253 (ramoops: Move to fs/pstore/ram.c).
> >
> > CC: [email protected]
> > Signed-off-by: Maxime Bizon <[email protected]>
> > Signed-off-by: Florian Fainelli <[email protected]>
>
> Thanks for the fix!
>
> Acked-by: Kees Cook <[email protected]>

Applied, thanks!