Skip to contentSkip to conversation

tracing: Fix unmapping loop in tracing_mark_write

PATCH In review Vikram Mulukutla 3 participants 3 messages Dec 18, 2014 – Feb 9, 2015

Subsystems (2)
Arm Qualcomm mailing list Tracing
lore
Share & tools
lore

Vikram Mulukutla

author
#0 lore raw

PATCH 1/1 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(-)
kernel/trace/trace.c +1 -1
index ab76b7b..bceed34 100644
@@ -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]);
 	}
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw
Quoted 6 lines · Vikram Mulukutla

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.

Quoted 7 lines

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?

Quoted 16 lines
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace.c +1 -1
index ab76b7b..bceed34 100644
@@ -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]);
 	}
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#2 lore raw
Quoted 10 lines · On Mon, 09 Feb 2015 15:33:58 -0800 Stephen Boyd <[email protected]> wrote:
Quoted 9 lines · Vikram Mulukutla

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.

Quoted 6 lines
Quoted 4 lines
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

Quoted 19 lines
Quoted 17 lines
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace.c +1 -1
index ab76b7b..bceed34 100644
@@ -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]);
 	}