Commit af4ee5e977acb150371c28bd85cb7e34cac48b13
Committed by
Linus Torvalds
1 parent
b1b00a5b8a
Exists in
ti-lsk-linux-4.1.y
and in
10 other branches
zsmalloc: correct fragile [kmap|kunmap]_atomic use
The kunmap_atomic should use virtual address getting by kmap_atomic. However, some pieces of code in zsmalloc uses modified address, not the one got by kmap_atomic for kunmap_atomic. It's okay for working because zsmalloc modifies the address inner PAGE_SIZE bounday so it works with current kmap_atomic's implementation. But it's still fragile with potential changing of kmap_atomic so let's correct it. I got a subtle bug when I implemented a new feature of zsmalloc (compaction) due to a link's mishandling (the link was over page boundary). Although it was totally my mistake, it took a while to find the cause because an unpredictable kmapped address was unmapped causing an almost random crash. Signed-off-by: Minchan Kim <minchan@kernel.org> Cc: Nitin Gupta <ngupta@vflare.org> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Cc: Dan Streetman <ddstreet@ieee.org> Cc: Seth Jennings <sjennings@variantweb.net> Cc: Jerome Marchand <jmarchan@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 1 changed file with 12 additions and 9 deletions Side-by-side Diff
mm/zsmalloc.c
... | ... | @@ -629,6 +629,7 @@ |
629 | 629 | struct page *next_page; |
630 | 630 | struct link_free *link; |
631 | 631 | unsigned int i = 1; |
632 | + void *vaddr; | |
632 | 633 | |
633 | 634 | /* |
634 | 635 | * page->index stores offset of first object starting |
... | ... | @@ -639,8 +640,8 @@ |
639 | 640 | if (page != first_page) |
640 | 641 | page->index = off; |
641 | 642 | |
642 | - link = (struct link_free *)kmap_atomic(page) + | |
643 | - off / sizeof(*link); | |
643 | + vaddr = kmap_atomic(page); | |
644 | + link = (struct link_free *)vaddr + off / sizeof(*link); | |
644 | 645 | |
645 | 646 | while ((off += class->size) < PAGE_SIZE) { |
646 | 647 | link->next = obj_location_to_handle(page, i++); |
... | ... | @@ -654,7 +655,7 @@ |
654 | 655 | */ |
655 | 656 | next_page = get_next_page(page); |
656 | 657 | link->next = obj_location_to_handle(next_page, 0); |
657 | - kunmap_atomic(link); | |
658 | + kunmap_atomic(vaddr); | |
658 | 659 | page = next_page; |
659 | 660 | off %= PAGE_SIZE; |
660 | 661 | } |
... | ... | @@ -1064,6 +1065,7 @@ |
1064 | 1065 | unsigned long obj; |
1065 | 1066 | struct link_free *link; |
1066 | 1067 | struct size_class *class; |
1068 | + void *vaddr; | |
1067 | 1069 | |
1068 | 1070 | struct page *first_page, *m_page; |
1069 | 1071 | unsigned long m_objidx, m_offset; |
1070 | 1072 | |
... | ... | @@ -1092,11 +1094,11 @@ |
1092 | 1094 | obj_handle_to_location(obj, &m_page, &m_objidx); |
1093 | 1095 | m_offset = obj_idx_to_offset(m_page, m_objidx, class->size); |
1094 | 1096 | |
1095 | - link = (struct link_free *)kmap_atomic(m_page) + | |
1096 | - m_offset / sizeof(*link); | |
1097 | + vaddr = kmap_atomic(m_page); | |
1098 | + link = (struct link_free *)vaddr + m_offset / sizeof(*link); | |
1097 | 1099 | first_page->freelist = link->next; |
1098 | 1100 | memset(link, POISON_INUSE, sizeof(*link)); |
1099 | - kunmap_atomic(link); | |
1101 | + kunmap_atomic(vaddr); | |
1100 | 1102 | |
1101 | 1103 | first_page->inuse++; |
1102 | 1104 | /* Now move the zspage to another fullness group, if required */ |
... | ... | @@ -1112,6 +1114,7 @@ |
1112 | 1114 | struct link_free *link; |
1113 | 1115 | struct page *first_page, *f_page; |
1114 | 1116 | unsigned long f_objidx, f_offset; |
1117 | + void *vaddr; | |
1115 | 1118 | |
1116 | 1119 | int class_idx; |
1117 | 1120 | struct size_class *class; |
1118 | 1121 | |
... | ... | @@ -1130,10 +1133,10 @@ |
1130 | 1133 | spin_lock(&class->lock); |
1131 | 1134 | |
1132 | 1135 | /* Insert this object in containing zspage's freelist */ |
1133 | - link = (struct link_free *)((unsigned char *)kmap_atomic(f_page) | |
1134 | - + f_offset); | |
1136 | + vaddr = kmap_atomic(f_page); | |
1137 | + link = (struct link_free *)(vaddr + f_offset); | |
1135 | 1138 | link->next = first_page->freelist; |
1136 | - kunmap_atomic(link); | |
1139 | + kunmap_atomic(vaddr); | |
1137 | 1140 | first_page->freelist = (void *)obj; |
1138 | 1141 | |
1139 | 1142 | first_page->inuse--; |