From: Linus Torvalds <torvalds@linux-foundation.org> To: Kees Cook <keescook@chromium.org> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Christoph Hellwig <hch@lst.de>, David Hildenbrand <david@redhat.com>, Hari Bathini <hbathini@linux.ibm.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Logan Gunthorpe <logang@deltatee.com>, Martin Oliveira <martin.oliveira@eideticom.com>, "Matthew Wilcox (Oracle)" <willy@infradead.org>, Mike Kravetz <mike.kravetz@oracle.com>, Minchan Kim <minchan@kernel.org>, Russell King <linux@armlinux.org.uk>, Stephen Rothwell <sfr@canb.auug.org.au>, Zi Yan <ziy@nvidia.com> Subject: Re: [GIT PULL] hardening fixes for v5.18-rc1 Date: Thu, 31 Mar 2022 11:49:42 -0700 [thread overview] Message-ID: <CAHk-=wjQ0+9jBy6bm850h1jms1tja8xnon4X5v0LSO4biLhFGg@mail.gmail.com> (raw) In-Reply-To: <202203311127.503A3110@keescook> [-- Attachment #1: Type: text/plain, Size: 560 bytes --] On Thu, Mar 31, 2022 at 11:35 AM Kees Cook <keescook@chromium.org> wrote: > > Please pull these hardening fixes for v5.18-rc1. This addresses an > -Warray-bounds warning found under a few ARM defconfigs, and disables > long-broken CONFIG_HARDENED_USERCOPY_PAGESPAN. Can't we just remove that HARDENED_USERCOPY_PAGESPAN thing entirely? Yes, yes, I know Matthew did that as part of other patches that is too late to go in any more in this merge window, but just the removal patch is a no-brainer. IOW, why not just do the attached? Linus [-- Attachment #2: patch.diff --] [-- Type: text/x-patch, Size: 4460 bytes --] arch/powerpc/configs/skiroot_defconfig | 1 - mm/usercopy.c | 67 ---------------------------------- security/Kconfig | 11 ------ 3 files changed, 79 deletions(-) diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig index f491875700e8..64176cc12d0e 100644 --- a/arch/powerpc/configs/skiroot_defconfig +++ b/arch/powerpc/configs/skiroot_defconfig @@ -274,7 +274,6 @@ CONFIG_NLS_UTF8=y CONFIG_ENCRYPTED_KEYS=y CONFIG_SECURITY=y CONFIG_HARDENED_USERCOPY=y -CONFIG_HARDENED_USERCOPY_PAGESPAN=y CONFIG_FORTIFY_SOURCE=y CONFIG_SECURITY_LOCKDOWN_LSM=y CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y diff --git a/mm/usercopy.c b/mm/usercopy.c index 2c235d5c2364..1ad8c755850b 100644 --- a/mm/usercopy.c +++ b/mm/usercopy.c @@ -157,70 +157,6 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n, usercopy_abort("null address", NULL, to_user, ptr, n); } -/* Checks for allocs that are marked in some way as spanning multiple pages. */ -static inline void check_page_span(const void *ptr, unsigned long n, - struct page *page, bool to_user) -{ -#ifdef CONFIG_HARDENED_USERCOPY_PAGESPAN - const void *end = ptr + n - 1; - struct page *endpage; - bool is_reserved, is_cma; - - /* - * Sometimes the kernel data regions are not marked Reserved (see - * check below). And sometimes [_sdata,_edata) does not cover - * rodata and/or bss, so check each range explicitly. - */ - - /* Allow reads of kernel rodata region (if not marked as Reserved). */ - if (ptr >= (const void *)__start_rodata && - end <= (const void *)__end_rodata) { - if (!to_user) - usercopy_abort("rodata", NULL, to_user, 0, n); - return; - } - - /* Allow kernel data region (if not marked as Reserved). */ - if (ptr >= (const void *)_sdata && end <= (const void *)_edata) - return; - - /* Allow kernel bss region (if not marked as Reserved). */ - if (ptr >= (const void *)__bss_start && - end <= (const void *)__bss_stop) - return; - - /* Is the object wholly within one base page? */ - if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) == - ((unsigned long)end & (unsigned long)PAGE_MASK))) - return; - - /* Allow if fully inside the same compound (__GFP_COMP) page. */ - endpage = virt_to_head_page(end); - if (likely(endpage == page)) - return; - - /* - * Reject if range is entirely either Reserved (i.e. special or - * device memory), or CMA. Otherwise, reject since the object spans - * several independently allocated pages. - */ - is_reserved = PageReserved(page); - is_cma = is_migrate_cma_page(page); - if (!is_reserved && !is_cma) - usercopy_abort("spans multiple pages", NULL, to_user, 0, n); - - for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) { - page = virt_to_head_page(ptr); - if (is_reserved && !PageReserved(page)) - usercopy_abort("spans Reserved and non-Reserved pages", - NULL, to_user, 0, n); - if (is_cma && !is_migrate_cma_page(page)) - usercopy_abort("spans CMA and non-CMA pages", NULL, - to_user, 0, n); - } -#endif -} - static inline void check_heap_object(const void *ptr, unsigned long n, bool to_user) { @@ -239,9 +175,6 @@ static inline void check_heap_object(const void *ptr, unsigned long n, if (folio_test_slab(folio)) { /* Check slab allocator for flags and size. */ __check_heap_object(ptr, n, folio_slab(folio), to_user); - } else { - /* Verify object does not incorrectly span multiple pages. */ - check_page_span(ptr, n, folio_page(folio, 0), to_user); } } diff --git a/security/Kconfig b/security/Kconfig index 9b2c4925585a..7d639f1b0c4a 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -163,17 +163,6 @@ config HARDENED_USERCOPY or are part of the kernel text. This kills entire classes of heap overflow exploits and similar kernel memory exposures. -config HARDENED_USERCOPY_PAGESPAN - bool "Refuse to copy allocations that span multiple pages" - depends on HARDENED_USERCOPY - depends on BROKEN - help - When a multi-page allocation is done without __GFP_COMP, - hardened usercopy will reject attempts to copy it. There are, - however, several cases of this in the kernel that have not all - been removed. This config is intended to be used only while - trying to find such users. - config FORTIFY_SOURCE bool "Harden common str/mem functions against buffer overflows" depends on ARCH_HAS_FORTIFY_SOURCE
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org> To: Kees Cook <keescook@chromium.org> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Christoph Hellwig <hch@lst.de>, David Hildenbrand <david@redhat.com>, Hari Bathini <hbathini@linux.ibm.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Logan Gunthorpe <logang@deltatee.com>, Martin Oliveira <martin.oliveira@eideticom.com>, "Matthew Wilcox (Oracle)" <willy@infradead.org>, Mike Kravetz <mike.kravetz@oracle.com>, Minchan Kim <minchan@kernel.org>, Russell King <linux@armlinux.org.uk>, Stephen Rothwell <sfr@canb.auug.org.au>, Zi Yan <ziy@nvidia.com> Subject: Re: [GIT PULL] hardening fixes for v5.18-rc1 Date: Thu, 31 Mar 2022 11:49:42 -0700 [thread overview] Message-ID: <CAHk-=wjQ0+9jBy6bm850h1jms1tja8xnon4X5v0LSO4biLhFGg@mail.gmail.com> (raw) In-Reply-To: <202203311127.503A3110@keescook> [-- Attachment #1: Type: text/plain, Size: 560 bytes --] On Thu, Mar 31, 2022 at 11:35 AM Kees Cook <keescook@chromium.org> wrote: > > Please pull these hardening fixes for v5.18-rc1. This addresses an > -Warray-bounds warning found under a few ARM defconfigs, and disables > long-broken CONFIG_HARDENED_USERCOPY_PAGESPAN. Can't we just remove that HARDENED_USERCOPY_PAGESPAN thing entirely? Yes, yes, I know Matthew did that as part of other patches that is too late to go in any more in this merge window, but just the removal patch is a no-brainer. IOW, why not just do the attached? Linus [-- Attachment #2: patch.diff --] [-- Type: text/x-patch, Size: 4460 bytes --] arch/powerpc/configs/skiroot_defconfig | 1 - mm/usercopy.c | 67 ---------------------------------- security/Kconfig | 11 ------ 3 files changed, 79 deletions(-) diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig index f491875700e8..64176cc12d0e 100644 --- a/arch/powerpc/configs/skiroot_defconfig +++ b/arch/powerpc/configs/skiroot_defconfig @@ -274,7 +274,6 @@ CONFIG_NLS_UTF8=y CONFIG_ENCRYPTED_KEYS=y CONFIG_SECURITY=y CONFIG_HARDENED_USERCOPY=y -CONFIG_HARDENED_USERCOPY_PAGESPAN=y CONFIG_FORTIFY_SOURCE=y CONFIG_SECURITY_LOCKDOWN_LSM=y CONFIG_SECURITY_LOCKDOWN_LSM_EARLY=y diff --git a/mm/usercopy.c b/mm/usercopy.c index 2c235d5c2364..1ad8c755850b 100644 --- a/mm/usercopy.c +++ b/mm/usercopy.c @@ -157,70 +157,6 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n, usercopy_abort("null address", NULL, to_user, ptr, n); } -/* Checks for allocs that are marked in some way as spanning multiple pages. */ -static inline void check_page_span(const void *ptr, unsigned long n, - struct page *page, bool to_user) -{ -#ifdef CONFIG_HARDENED_USERCOPY_PAGESPAN - const void *end = ptr + n - 1; - struct page *endpage; - bool is_reserved, is_cma; - - /* - * Sometimes the kernel data regions are not marked Reserved (see - * check below). And sometimes [_sdata,_edata) does not cover - * rodata and/or bss, so check each range explicitly. - */ - - /* Allow reads of kernel rodata region (if not marked as Reserved). */ - if (ptr >= (const void *)__start_rodata && - end <= (const void *)__end_rodata) { - if (!to_user) - usercopy_abort("rodata", NULL, to_user, 0, n); - return; - } - - /* Allow kernel data region (if not marked as Reserved). */ - if (ptr >= (const void *)_sdata && end <= (const void *)_edata) - return; - - /* Allow kernel bss region (if not marked as Reserved). */ - if (ptr >= (const void *)__bss_start && - end <= (const void *)__bss_stop) - return; - - /* Is the object wholly within one base page? */ - if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) == - ((unsigned long)end & (unsigned long)PAGE_MASK))) - return; - - /* Allow if fully inside the same compound (__GFP_COMP) page. */ - endpage = virt_to_head_page(end); - if (likely(endpage == page)) - return; - - /* - * Reject if range is entirely either Reserved (i.e. special or - * device memory), or CMA. Otherwise, reject since the object spans - * several independently allocated pages. - */ - is_reserved = PageReserved(page); - is_cma = is_migrate_cma_page(page); - if (!is_reserved && !is_cma) - usercopy_abort("spans multiple pages", NULL, to_user, 0, n); - - for (ptr += PAGE_SIZE; ptr <= end; ptr += PAGE_SIZE) { - page = virt_to_head_page(ptr); - if (is_reserved && !PageReserved(page)) - usercopy_abort("spans Reserved and non-Reserved pages", - NULL, to_user, 0, n); - if (is_cma && !is_migrate_cma_page(page)) - usercopy_abort("spans CMA and non-CMA pages", NULL, - to_user, 0, n); - } -#endif -} - static inline void check_heap_object(const void *ptr, unsigned long n, bool to_user) { @@ -239,9 +175,6 @@ static inline void check_heap_object(const void *ptr, unsigned long n, if (folio_test_slab(folio)) { /* Check slab allocator for flags and size. */ __check_heap_object(ptr, n, folio_slab(folio), to_user); - } else { - /* Verify object does not incorrectly span multiple pages. */ - check_page_span(ptr, n, folio_page(folio, 0), to_user); } } diff --git a/security/Kconfig b/security/Kconfig index 9b2c4925585a..7d639f1b0c4a 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -163,17 +163,6 @@ config HARDENED_USERCOPY or are part of the kernel text. This kills entire classes of heap overflow exploits and similar kernel memory exposures. -config HARDENED_USERCOPY_PAGESPAN - bool "Refuse to copy allocations that span multiple pages" - depends on HARDENED_USERCOPY - depends on BROKEN - help - When a multi-page allocation is done without __GFP_COMP, - hardened usercopy will reject attempts to copy it. There are, - however, several cases of this in the kernel that have not all - been removed. This config is intended to be used only while - trying to find such users. - config FORTIFY_SOURCE bool "Harden common str/mem functions against buffer overflows" depends on ARCH_HAS_FORTIFY_SOURCE [-- Attachment #3: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-03-31 18:50 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-31 18:35 [GIT PULL] hardening fixes for v5.18-rc1 Kees Cook 2022-03-31 18:35 ` Kees Cook 2022-03-31 18:46 ` Russell King (Oracle) 2022-03-31 18:46 ` Russell King (Oracle) 2022-03-31 18:57 ` Kees Cook 2022-03-31 18:57 ` Kees Cook 2022-03-31 18:49 ` Linus Torvalds [this message] 2022-03-31 18:49 ` Linus Torvalds 2022-03-31 19:00 ` Kees Cook 2022-03-31 19:00 ` Kees Cook 2022-03-31 19:09 ` Linus Torvalds 2022-03-31 19:09 ` Linus Torvalds 2022-03-31 19:12 ` pr-tracker-bot 2022-03-31 19:12 ` pr-tracker-bot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHk-=wjQ0+9jBy6bm850h1jms1tja8xnon4X5v0LSO4biLhFGg@mail.gmail.com' \ --to=torvalds@linux-foundation.org \ --cc=akpm@linux-foundation.org \ --cc=david@redhat.com \ --cc=hbathini@linux.ibm.com \ --cc=hch@lst.de \ --cc=keescook@chromium.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=logang@deltatee.com \ --cc=martin.oliveira@eideticom.com \ --cc=mike.kravetz@oracle.com \ --cc=minchan@kernel.org \ --cc=sfr@canb.auug.org.au \ --cc=willy@infradead.org \ --cc=ziy@nvidia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.