[PATCH openEuler-1.0-LTS 1/2] hugetlb: before freeing hugetlb page set dtor to appropriate value
From: Mike Kravetz mike.kravetz@oracle.com
mainline inclusion from mainline-5.15-rc1 commit e32d20c0c88b1cd0a44f882c4f0eb2f536363d1b category: bugfix bugzilla: 180680 CVE: NA
---------------------------
When removing a hugetlb page from the pool the ref count is set to one (as the free page has no ref count) and compound page destructor is set to NULL_COMPOUND_DTOR. Since a subsequent call to free the hugetlb page will call __free_pages for non-gigantic pages and free_gigantic_page for gigantic pages the destructor is not used.
However, consider the following race with code taking a speculative reference on the page:
Thread 0 Thread 1 -------- -------- remove_hugetlb_page set_page_refcounted(page); set_compound_page_dtor(page, NULL_COMPOUND_DTOR); get_page_unless_zero(page) __update_and_free_page __free_pages(page, huge_page_order(h));
/* Note that __free_pages() will simply drop the reference to the page. */
put_page(page) __put_compound_page() destroy_compound_page NULL_COMPOUND_DTOR BUG: kernel NULL pointer dereference, address: 0000000000000000
To address this race, set the dtor to the normal compound page dtor for non-gigantic pages. The dtor for gigantic pages does not matter as gigantic pages are changed from a compound page to 'just a group of pages' before freeing. Hence, the destructor is not used.
Link: https://lkml.kernel.org/r/20210809184832.18342-4-mike.kravetz@oracle.com Signed-off-by: Mike Kravetz mike.kravetz@oracle.com Reviewed-by: Muchun Song songmuchun@bytedance.com Cc: Michal Hocko mhocko@suse.com Cc: Oscar Salvador osalvador@suse.de Cc: David Hildenbrand david@redhat.com Cc: Matthew Wilcox willy@infradead.org Cc: Naoya Horiguchi naoya.horiguchi@linux.dev Cc: Mina Almasry almasrymina@google.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Conflicts: mm/hugetlb.c Signed-off-by: Chen Wandun chenwandun@huawei.com Reviewed-by: Kefeng Wang wangkefeng.wang@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- mm/hugetlb.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 5190154de3b09..2f65dad443ab3 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1213,12 +1213,30 @@ static void update_and_free_page(struct hstate *h, struct page *page) 1 << PG_writeback); } VM_BUG_ON_PAGE(hugetlb_cgroup_from_page(page), page); - set_compound_page_dtor(page, NULL_COMPOUND_DTOR); + /* + * Very subtle + * + * For non-gigantic pages set the destructor to the normal compound + * page dtor. This is needed in case someone takes an additional + * temporary ref to the page, and freeing is delayed until they drop + * their reference. + * + * For gigantic pages set the destructor to the null dtor. This + * destructor will never be called. Before freeing the gigantic + * page destroy_compound_gigantic_page will turn the compound page + * into a simple group of pages. After this the destructor does not + * apply. + * + * This handles the case where more than one ref is held when and + * after update_and_free_page is called. + */ set_page_refcounted(page); if (hstate_is_gigantic(h)) { + set_compound_page_dtor(page, NULL_COMPOUND_DTOR); destroy_compound_gigantic_page(page, huge_page_order(h)); free_gigantic_page(page, huge_page_order(h)); } else { + set_compound_page_dtor(page, COMPOUND_PAGE_DTOR); __free_pages(page, huge_page_order(h)); } }
From: Robin Murphy robin.murphy@arm.com
mainline inclusion from mainline-5.14-rc2 commit 295cf156231ca3f9e3a66bde7fab5e09c41835e0 category: bugfix bugzilla: 55085 CVE: NA
---------------------------
Al reminds us that the usercopy API must only return complete failure if absolutely nothing could be copied. Currently, if userspace does something silly like giving us an unaligned pointer to Device memory, or a size which overruns MTE tag bounds, we may fail to honour that requirement when faulting on a multi-byte access even though a smaller access could have succeeded.
Add a mitigation to the fixup routines to fall back to a single-byte copy if we faulted on a larger access before anything has been written to the destination, to guarantee making *some* forward progress. We needn't be too concerned about the overall performance since this should only occur when callers are doing something a bit dodgy in the first place. Particularly broken userspace might still be able to trick generic_perform_write() into an infinite loop by targeting write() at an mmap() of some read-only device register where the fault-in load succeeds but any store synchronously aborts such that copy_to_user() is genuinely unable to make progress, but, well, don't do that...
CC: stable@vger.kernel.org Reported-by: Chen Huang chenhuang5@huawei.com Suggested-by: Al Viro viro@zeniv.linux.org.uk Reviewed-by: Catalin Marinas catalin.marinas@arm.com Signed-off-by: Robin Murphy robin.murphy@arm.com Link: https://lore.kernel.org/r/dc03d5c675731a1f24a62417dba5429ad744234e.162609843... Signed-off-by: Will Deacon will@kernel.org
Conflicts: arch/arm64/lib/copy_from_user.S arch/arm64/lib/copy_in_user.S arch/arm64/lib/copy_to_user.S Signed-off-by: Chen Huang chenhuang5@huawei.com Reviewed-by: Chen Wandun chenwandun@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- arch/arm64/lib/copy_from_user.S | 13 ++++++++++--- arch/arm64/lib/copy_in_user.S | 20 ++++++++++++++------ arch/arm64/lib/copy_to_user.S | 14 +++++++++++--- 3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S index 96b22c0fa3432..7cd6eeaa216cf 100644 --- a/arch/arm64/lib/copy_from_user.S +++ b/arch/arm64/lib/copy_from_user.S @@ -39,7 +39,7 @@ .endm
.macro ldrh1 ptr, regB, val - uao_user_alternative 9998f, ldrh, ldtrh, \ptr, \regB, \val + uao_user_alternative 9997f, ldrh, ldtrh, \ptr, \regB, \val .endm
.macro strh1 ptr, regB, val @@ -47,7 +47,7 @@ .endm
.macro ldr1 ptr, regB, val - uao_user_alternative 9998f, ldr, ldtr, \ptr, \regB, \val + uao_user_alternative 9997f, ldr, ldtr, \ptr, \regB, \val .endm
.macro str1 ptr, regB, val @@ -55,7 +55,7 @@ .endm
.macro ldp1 ptr, regB, regC, val - uao_ldp 9998f, \ptr, \regB, \regC, \val + uao_ldp 9997f, \ptr, \regB, \regC, \val .endm
.macro stp1 ptr, regB, regC, val @@ -63,9 +63,11 @@ .endm
end .req x5 +srcin .req x15 ENTRY(__arch_copy_from_user) uaccess_enable_not_uao x3, x4, x5 add end, x0, x2 + mov srcin, x1 #include "copy_template.S" uaccess_disable_not_uao x3, x4 mov x0, #0 // Nothing to copy @@ -74,6 +76,11 @@ ENDPROC(__arch_copy_from_user)
.section .fixup,"ax" .align 2 +9997: cmp dst, dstin + b.ne 9998f + // Before being absolutely sure we couldn't copy anything, try harder +USER(9998f, ldtrb tmp1w, [srcin]) + strb tmp1w, [dst], #1 9998: sub x0, end, dst // bytes not copied uaccess_disable_not_uao x3, x4 ret diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S index e56c705f1f236..b20d3a0b32374 100644 --- a/arch/arm64/lib/copy_in_user.S +++ b/arch/arm64/lib/copy_in_user.S @@ -40,34 +40,36 @@ .endm
.macro ldrh1 ptr, regB, val - uao_user_alternative 9998f, ldrh, ldtrh, \ptr, \regB, \val + uao_user_alternative 9997f, ldrh, ldtrh, \ptr, \regB, \val .endm
.macro strh1 ptr, regB, val - uao_user_alternative 9998f, strh, sttrh, \ptr, \regB, \val + uao_user_alternative 9997f, strh, sttrh, \ptr, \regB, \val .endm
.macro ldr1 ptr, regB, val - uao_user_alternative 9998f, ldr, ldtr, \ptr, \regB, \val + uao_user_alternative 9997f, ldr, ldtr, \ptr, \regB, \val .endm
.macro str1 ptr, regB, val - uao_user_alternative 9998f, str, sttr, \ptr, \regB, \val + uao_user_alternative 9997f, str, sttr, \ptr, \regB, \val .endm
.macro ldp1 ptr, regB, regC, val - uao_ldp 9998f, \ptr, \regB, \regC, \val + uao_ldp 9997f, \ptr, \regB, \regC, \val .endm
.macro stp1 ptr, regB, regC, val - uao_stp 9998f, \ptr, \regB, \regC, \val + uao_stp 9997f, \ptr, \regB, \regC, \val .endm
end .req x5 +srcin .req x15
ENTRY(__arch_copy_in_user) uaccess_enable_not_uao x3, x4, x5 add end, x0, x2 + mov srcin, x1 #include "copy_template.S" uaccess_disable_not_uao x3, x4 mov x0, #0 @@ -76,6 +78,12 @@ ENDPROC(__arch_copy_in_user)
.section .fixup,"ax" .align 2 +9997: cmp dst, dstin + b.ne 9998f + // Before being absolutely sure we couldn't copy anything, try harder +USER(9998f, ldtrb tmp1w, [srcin]) +USER(9998f, sttrb tmp1w, [dst]) + add dst, dst, #1 9998: sub x0, end, dst // bytes not copied uaccess_disable_not_uao x3, x4 ret diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S index 769133236f4ad..84b7a16e82b5d 100644 --- a/arch/arm64/lib/copy_to_user.S +++ b/arch/arm64/lib/copy_to_user.S @@ -42,7 +42,7 @@ .endm
.macro strh1 ptr, regB, val - uao_user_alternative 9998f, strh, sttrh, \ptr, \regB, \val + uao_user_alternative 9997f, strh, sttrh, \ptr, \regB, \val .endm
.macro ldr1 ptr, regB, val @@ -50,7 +50,7 @@ .endm
.macro str1 ptr, regB, val - uao_user_alternative 9998f, str, sttr, \ptr, \regB, \val + uao_user_alternative 9997f, str, sttr, \ptr, \regB, \val .endm
.macro ldp1 ptr, regB, regC, val @@ -58,13 +58,15 @@ .endm
.macro stp1 ptr, regB, regC, val - uao_stp 9998f, \ptr, \regB, \regC, \val + uao_stp 9997f, \ptr, \regB, \regC, \val .endm
end .req x5 +srcin .req x15 ENTRY(__arch_copy_to_user) uaccess_enable_not_uao x3, x4, x5 add end, x0, x2 + mov srcin, x1 #include "copy_template.S" uaccess_disable_not_uao x3, x4 mov x0, #0 @@ -92,6 +94,12 @@ ENDPROC(__arch_copy_to_user_generic_read)
.section .fixup,"ax" .align 2 +9997: cmp dst, dstin + b.ne 9998f + // Before being absolutely sure we couldn't copy anything, try harder + ldrb tmp1w, [srcin] +USER(9998f, sttrb tmp1w, [dst]) + add dst, dst, #1 9998: sub x0, end, dst // bytes not copied uaccess_disable_not_uao x3, x4 ret
-
Yang Yingliang