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
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
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
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
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
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!