2014-12-18 02:51:14

by Vikram Mulukutla

Subject: [PATCH] tracing: Fix unmapping loop in tracing_mark_write

Commit 6edb2a8a385f0cdef51dae37ff23e74d76d8a6ce introduced
an array map_pages that contains the addresses returned by
kmap_atomic. However, when unmapping those pages, map_pages[0]
is unmapped before map_pages[1], breaking the nesting requirement
as specified in the documentation for kmap_atomic/kunmap_atomic.

This was caught by the highmem debug code present in kunmap_atomic.
Fix the loop to do the unmapping properly.

Reviewed-by: Stephen Boyd <[email protected]>
Reported-by: Lime Yang <[email protected]>
Signed-off-by: Vikram Mulukutla <[email protected]>
---
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ab76b7b..bceed34 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4931,7 +4931,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
*fpos += written;

out_unlock:
- for (i = 0; i < nr_pages; i++){
+ for (i = nr_pages - 1; i >= 0; i--) {
kunmap_atomic(map_page[i]);
put_page(pages[i]);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-02-09 23:34:02

by Stephen Boyd

Subject: Re: [PATCH] tracing: Fix unmapping loop in tracing_mark_write

On 12/17/14 18:50, Vikram Mulukutla wrote:
> Commit 6edb2a8a385f0cdef51dae37ff23e74d76d8a6ce introduced
> an array map_pages that contains the addresses returned by
> kmap_atomic. However, when unmapping those pages, map_pages[0]
> is unmapped before map_pages[1], breaking the nesting requirement
> as specified in the documentation for kmap_atomic/kunmap_atomic.
>
> This was caught by the highmem debug code present in kunmap_atomic.
> Fix the loop to do the unmapping properly.
>
> Reviewed-by: Stephen Boyd <[email protected]>
> Reported-by: Lime Yang <[email protected]>
> Signed-off-by: Vikram Mulukutla <[email protected]>
> ---

ping? What happened to this patch?

> kernel/trace/trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ab76b7b..bceed34 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4931,7 +4931,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> *fpos += written;
>
> out_unlock:
> - for (i = 0; i < nr_pages; i++){
> + for (i = nr_pages - 1; i >= 0; i--) {
> kunmap_atomic(map_page[i]);
> put_page(pages[i]);
> }


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-02-09 23:45:58

by Steven Rostedt

Subject: Re: [PATCH] tracing: Fix unmapping loop in tracing_mark_write

On Mon, 09 Feb 2015 15:33:58 -0800
Stephen Boyd <[email protected]> wrote:

> On 12/17/14 18:50, Vikram Mulukutla wrote:
> > Commit 6edb2a8a385f0cdef51dae37ff23e74d76d8a6ce introduced
> > an array map_pages that contains the addresses returned by
> > kmap_atomic. However, when unmapping those pages, map_pages[0]
> > is unmapped before map_pages[1], breaking the nesting requirement
> > as specified in the documentation for kmap_atomic/kunmap_atomic.
> >
> > This was caught by the highmem debug code present in kunmap_atomic.
> > Fix the loop to do the unmapping properly.
> >
> > Reviewed-by: Stephen Boyd <[email protected]>
> > Reported-by: Lime Yang <[email protected]>
> > Signed-off-by: Vikram Mulukutla <[email protected]>
> > ---
>
> ping? What happened to this patch?

Seems to have been lost in my INBOX.

Thanks for pointing it out. I'll add it to my 3.20 queue. Looks like it
should be marked for stable as well. As far back as 3.5 as I can tell.

-- Steve

>
> > kernel/trace/trace.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index ab76b7b..bceed34 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -4931,7 +4931,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> > *fpos += written;
> >
> > out_unlock:
> > - for (i = 0; i < nr_pages; i++){
> > + for (i = nr_pages - 1; i >= 0; i--) {
> > kunmap_atomic(map_page[i]);
> > put_page(pages[i]);
> > }
>
>