* Re: Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
@ 2016-07-09 5:58 ` Michael Ellerman
2016-07-09 5:58 ` [kernel-hardening] " Michael Ellerman
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 5:58 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: kernel-hardening, Jan Kara, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, linux-arch,
x86, Russell King, PaX Team, Borislav Petkov, Mathias Krause,
Fenghua Yu, Rik van Riel, David Rientjes, Tony Luck,
Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov, Laura Abb ott,
Brad Spengler, Ard Biesheuvel
Kees Cook <keescook@chromium.org> writes:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> So, as found already, the position in the usercopy check needs to be
>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>> include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
That fixes the case for me in kstrndup(), which allows the system to boot.
I then get two hits, which may or may not be valid:
[ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
[ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.310480] Call Trace:
[ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
[ 2.317045] usercopy: kernel memory overwrite attempt detected to d000000003530028 (kernfs_node_cache) (64 bytes)
[ 2.317297] CPU: 10 PID: 2242 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.317475] Call Trace:
[ 2.317511] [c0000001f471fbf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.317689] [c0000001f471fc30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.317861] [c0000001f471fcb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.318011] [c0000001f471fcf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.318165] [c0000001f471fd30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.318313] [c0000001f471fd60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.318485] [c0000001f471fdd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.318632] [c0000001f471fe30] [c000000000009260] system_call+0x38/0x108
With:
# zgrep SLUB /proc/config.gz
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_CPU_PARTIAL=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
cheers
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
2016-07-09 5:58 ` Michael Ellerman
@ 2016-07-09 5:58 ` Michael Ellerman
2016-07-09 5:58 ` Michael Ellerman
` (5 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 5:58 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: kernel-hardening, Jan Kara, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, linux-arch,
x86, Russell King, PaX Team, Borislav Petkov, Mathias Krause,
Fenghua Yu, Rik van Riel, David Rientjes, Tony Luck,
Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov, Laura Abbott,
Brad Spengler, Ard Biesheuvel, LKML, Pekka Enberg,
Case y Sc hauf ler, Andrew Morton, linuxppc-dev, David S. Miller,
linux-arm-kernel
Kees Cook <keescook@chromium.org> writes:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> So, as found already, the position in the usercopy check needs to be
>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>> include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
That fixes the case for me in kstrndup(), which allows the system to boot.
I then get two hits, which may or may not be valid:
[ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
[ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.310480] Call Trace:
[ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
[ 2.317045] usercopy: kernel memory overwrite attempt detected to d000000003530028 (kernfs_node_cache) (64 bytes)
[ 2.317297] CPU: 10 PID: 2242 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.317475] Call Trace:
[ 2.317511] [c0000001f471fbf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.317689] [c0000001f471fc30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.317861] [c0000001f471fcb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.318011] [c0000001f471fcf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.318165] [c0000001f471fd30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.318313] [c0000001f471fd60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.318485] [c0000001f471fdd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.318632] [c0000001f471fe30] [c000000000009260] system_call+0x38/0x108
With:
# zgrep SLUB /proc/config.gz
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_CPU_PARTIAL=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
cheers
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
2016-07-09 5:58 ` Michael Ellerman
2016-07-09 5:58 ` [kernel-hardening] " Michael Ellerman
@ 2016-07-09 5:58 ` Michael Ellerman
2016-07-09 5:58 ` Michael Ellerman
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 5:58 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Kees Cook <keescook@chromium.org> writes:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> So, as found already, the position in the usercopy check needs to be
>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>> include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
That fixes the case for me in kstrndup(), which allows the system to boot.
I then get two hits, which may or may not be valid:
[ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
[ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.310480] Call Trace:
[ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
[ 2.317045] usercopy: kernel memory overwrite attempt detected to d000000003530028 (kernfs_node_cache) (64 bytes)
[ 2.317297] CPU: 10 PID: 2242 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.317475] Call Trace:
[ 2.317511] [c0000001f471fbf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.317689] [c0000001f471fc30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.317861] [c0000001f471fcb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.318011] [c0000001f471fcf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.318165] [c0000001f471fd30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.318313] [c0000001f471fd60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.318485] [c0000001f471fdd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.318632] [c0000001f471fe30] [c000000000009260] system_call+0x38/0x108
With:
# zgrep SLUB /proc/config.gz
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_CPU_PARTIAL=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
` (2 preceding siblings ...)
2016-07-09 5:58 ` Michael Ellerman
@ 2016-07-09 5:58 ` Michael Ellerman
2016-07-09 5:58 ` Michael Ellerman
` (3 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 5:58 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Kees Cook <keescook@chromium.org> writes:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> So, as found already, the position in the usercopy check needs to be
>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>> include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
That fixes the case for me in kstrndup(), which allows the system to boot.
I then get two hits, which may or may not be valid:
[ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
[ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.310480] Call Trace:
[ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
[ 2.317045] usercopy: kernel memory overwrite attempt detected to d000000003530028 (kernfs_node_cache) (64 bytes)
[ 2.317297] CPU: 10 PID: 2242 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.317475] Call Trace:
[ 2.317511] [c0000001f471fbf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.317689] [c0000001f471fc30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.317861] [c0000001f471fcb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.318011] [c0000001f471fcf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.318165] [c0000001f471fd30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.318313] [c0000001f471fd60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.318485] [c0000001f471fdd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.318632] [c0000001f471fe30] [c000000000009260] system_call+0x38/0x108
With:
# zgrep SLUB /proc/config.gz
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_CPU_PARTIAL=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
` (3 preceding siblings ...)
2016-07-09 5:58 ` Michael Ellerman
@ 2016-07-09 5:58 ` Michael Ellerman
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 5:58 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Kees Cook <keescook@chromium.org> writes:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>> So, as found already, the position in the usercopy check needs to be
>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>> include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
That fixes the case for me in kstrndup(), which allows the system to boot.
I then get two hits, which may or may not be valid:
[ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
[ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.310480] Call Trace:
[ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
[ 2.317045] usercopy: kernel memory overwrite attempt detected to d000000003530028 (kernfs_node_cache) (64 bytes)
[ 2.317297] CPU: 10 PID: 2242 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
[ 2.317475] Call Trace:
[ 2.317511] [c0000001f471fbf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[ 2.317689] [c0000001f471fc30] [c00000000029cf44] __check_object_size+0x74/0x320
[ 2.317861] [c0000001f471fcb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[ 2.318011] [c0000001f471fcf0] [c0000000008b38f4] __get_filter+0x74/0x160
[ 2.318165] [c0000001f471fd30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[ 2.318313] [c0000001f471fd60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[ 2.318485] [c0000001f471fdd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[ 2.318632] [c0000001f471fe30] [c000000000009260] system_call+0x38/0x108
With:
# zgrep SLUB /proc/config.gz
CONFIG_SLUB_DEBUG=y
CONFIG_SLUB=y
CONFIG_SLUB_CPU_PARTIAL=y
CONFIG_SLUB_DEBUG_ON=y
# CONFIG_SLUB_STATS is not set
cheers
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <8737njpd37.fsf@@concordia.ellerman.id.au>]
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
@ 2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
` (3 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 6:07 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> + /* Adjust for redzone and reject if within the redzone. */
>> + if (s->flags & SLAB_RED_ZONE) {
>> + if (offset < s->red_left_pad)
>> + return s->name;
>> + offset -= s->red_left_pad;
>> + }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.
Ugh, no it doesn't, booted the wrong kernel.
I don't see the oops in strndup_user(), but instead get:
usercopy: kernel memory overwrite attempt detected to d000000003610028 (cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c0000001fb087bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c0000001fb087c30] [c00000000029cf44] __check_object_size+0x74/0x320
[c0000001fb087cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[c0000001fb087cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[c0000001fb087d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[c0000001fb087d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[c0000001fb087dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[c0000001fb087e30] [c000000000009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
cheers
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
2016-07-09 6:07 ` Michael Ellerman
@ 2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
` (2 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 6:07 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> + /* Adjust for redzone and reject if within the redzone. */
>> + if (s->flags & SLAB_RED_ZONE) {
>> + if (offset < s->red_left_pad)
>> + return s->name;
>> + offset -= s->red_left_pad;
>> + }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.
Ugh, no it doesn't, booted the wrong kernel.
I don't see the oops in strndup_user(), but instead get:
usercopy: kernel memory overwrite attempt detected to d000000003610028 (cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c0000001fb087bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c0000001fb087c30] [c00000000029cf44] __check_object_size+0x74/0x320
[c0000001fb087cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[c0000001fb087cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[c0000001fb087d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[c0000001fb087d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[c0000001fb087dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[c0000001fb087e30] [c000000000009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
@ 2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
4 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 6:07 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: kernel-hardening, Jan Kara, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, linux-arch,
x86, Russell King, PaX Team, Borislav Petkov, Mathias Krause,
Fenghua Yu, Rik van Riel, David Rientjes, Tony Luck,
Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov, Laura Abbott,
Brad Spengler, Ard Biesheuvel, LKML, Pekka Enberg,
Case y Sc hauf ler, Andrew Morton, linuxppc-dev, David S. Miller,
linux-arm-kernel
Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> + /* Adjust for redzone and reject if within the redzone. */
>> + if (s->flags & SLAB_RED_ZONE) {
>> + if (offset < s->red_left_pad)
>> + return s->name;
>> + offset -= s->red_left_pad;
>> + }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.
Ugh, no it doesn't, booted the wrong kernel.
I don't see the oops in strndup_user(), but instead get:
usercopy: kernel memory overwrite attempt detected to d000000003610028 (cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c0000001fb087bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c0000001fb087c30] [c00000000029cf44] __check_object_size+0x74/0x320
[c0000001fb087cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[c0000001fb087cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[c0000001fb087d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[c0000001fb087d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[c0000001fb087dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[c0000001fb087e30] [c000000000009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
cheers
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
` (2 preceding siblings ...)
2016-07-09 6:07 ` Michael Ellerman
@ 2016-07-09 6:07 ` Michael Ellerman
2016-07-09 6:07 ` Michael Ellerman
4 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 6:07 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: Jan Kara, kernel-hardening, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, x86,
Russell King, linux-arm-kernel, David Rientjes, PaX Team,
Borislav Petkov, Mathias Krause, linux-arch, Rik van Riel,
Brad Spengler, Andy Lutomirski, Andrew Morton, Dmitry Vyukov,
Laura Abbott
Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> + /* Adjust for redzone and reject if within the redzone. */
>> + if (s->flags & SLAB_RED_ZONE) {
>> + if (offset < s->red_left_pad)
>> + return s->name;
>> + offset -= s->red_left_pad;
>> + }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.
Ugh, no it doesn't, booted the wrong kernel.
I don't see the oops in strndup_user(), but instead get:
usercopy: kernel memory overwrite attempt detected to d000000003610028 (cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c0000001fb087bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c0000001fb087c30] [c00000000029cf44] __check_object_size+0x74/0x320
[c0000001fb087cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[c0000001fb087cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[c0000001fb087d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[c0000001fb087d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[c0000001fb087dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[c0000001fb087e30] [c000000000009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <8737njpd37.fsf@@concordia.ellerman.id.au>
` (3 preceding siblings ...)
2016-07-09 6:07 ` Michael Ellerman
@ 2016-07-09 6:07 ` Michael Ellerman
4 siblings, 0 replies; 22+ messages in thread
From: Michael Ellerman @ 2016-07-09 6:07 UTC (permalink / raw)
To: Kees Cook, Christoph Lameter
Cc: kernel-hardening, Jan Kara, Catalin Marinas, Will Deacon,
Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli, linux-arch,
x86, Russell King, PaX Team, Borislav Petkov, Mathias Krause,
Fenghua Yu, Rik van Riel, David Rientjes, Tony Luck,
Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov, Laura Abb ott,
Brad Spengler, Ard Biesheuvel
Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> + /* Adjust for redzone and reject if within the redzone. */
>> + if (s->flags & SLAB_RED_ZONE) {
>> + if (offset < s->red_left_pad)
>> + return s->name;
>> + offset -= s->red_left_pad;
>> + }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.
Ugh, no it doesn't, booted the wrong kernel.
I don't see the oops in strndup_user(), but instead get:
usercopy: kernel memory overwrite attempt detected to d000000003610028 (cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c0000001fb087bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c0000001fb087c30] [c00000000029cf44] __check_object_size+0x74/0x320
[c0000001fb087cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
[c0000001fb087cf0] [c0000000008b38f4] __get_filter+0x74/0x160
[c0000001fb087d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
[c0000001fb087d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
[c0000001fb087dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
[c0000001fb087e30] [c000000000009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
cheers
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <57809299.84b3370a.5390c.ffff9e58SMTPIN_ADDED_BROKEN@mx.google.com>]
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
[not found] ` <57809299.84b3370a.5390c.ffff9e58SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2016-07-09 6:17 ` Valdis.Kletnieks
2016-07-09 17:07 ` Kees Cook
0 siblings, 1 reply; 22+ messages in thread
From: Valdis.Kletnieks @ 2016-07-09 6:17 UTC (permalink / raw)
To: kernel-hardening
Cc: Kees Cook, Christoph Lameter, Jan Kara, Catalin Marinas,
Will Deacon, Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli,
linux-arch, x86, Russell King, PaX Team, Borislav Petkov,
Mathias Krause, Fenghua Yu, Rik van Riel, David Rientjes,
Tony Luck, Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov,
Laura Abbott, Brad Spengler, Ard Biesheuvel, LKML, Pekka Enberg,
Case y Sc hauf ler, Andrew Morton, linuxppc-dev, David S. Miller,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]
On Sat, 09 Jul 2016 15:58:20 +1000, Michael Ellerman said:
> I then get two hits, which may or may not be valid:
>
> [ 2.309556] usercopy: kernel memory overwrite attempt detected to d000000003510028 (kernfs_node_cache) (64 bytes)
> [ 2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 4.7.0-rc3-00099-g97872fc89d41 #64
> [ 2.310480] Call Trace:
> [ 2.310556] [c0000001f4773bf0] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
> [ 2.311016] [c0000001f4773c30] [c00000000029cf44] __check_object_size+0x74/0x320
> [ 2.311472] [c0000001f4773cb0] [c00000000005d4d0] copy_from_user+0x60/0xd4
> [ 2.311873] [c0000001f4773cf0] [c0000000008b38f4] __get_filter+0x74/0x160
> [ 2.312230] [c0000001f4773d30] [c0000000008b408c] sk_attach_filter+0x2c/0xc0
> [ 2.312596] [c0000001f4773d60] [c000000000871c34] sock_setsockopt+0x954/0xc00
> [ 2.313021] [c0000001f4773dd0] [c00000000086ac44] SyS_setsockopt+0x134/0x150
> [ 2.313380] [c0000001f4773e30] [c000000000009260] system_call+0x38/0x108
Yeah, 'ping' dies with a similar traceback going to rawv6_setsockopt(),
and 'trinity' dies a horrid death during initialization because it creates
some sctp sockets to fool around with. The problem in all these cases is that
setsockopt uses copy_from_user() to pull in the option value, and the allocation
isn't tagged with USERCOPY to whitelist it.
Unfortunately, I haven't been able to track down where in net/ the memory is
allocated, nor is there any good hint in the grsecurity patch that I can find
where they do the tagging.
And the fact that so far, I'm only had ping and trinity killed in setsockopt()
hints that *most* setsockopt() calls must be going through a code path that
does allocate suitable memory, and these two have different paths. I can't
believe they're the only two binaries that call setsockopt().....
Just saw your second mail, now I'm wondering why *my* laptop doesn't die a
horrid death when systemd starts up. Mine is
systemd-230-3.gitea68351.fc25.x86_64 - maybe there's something
release-dependent going on?
[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-09 6:17 ` [kernel-hardening] " Valdis.Kletnieks
@ 2016-07-09 17:07 ` Kees Cook
0 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2016-07-09 17:07 UTC (permalink / raw)
To: Valdis Kletnieks
Cc: kernel-hardening, Christoph Lameter, Jan Kara, Catalin Marinas,
Will Deacon, Linux-MM, sparclinux, linux-ia64, Andrea Arcangeli,
linux-arch, x86, Russell King, PaX Team, Borislav Petkov,
Mathias Krause, Fenghua Yu, Rik van Riel, David Rientjes,
Tony Luck, Andy Lutomirski, Joonsoo Kim, Dmitry Vyukov,
Laura Abbott, Brad Spengler, Ard Biesheuvel, LKML, Pekka Enberg,
Case y Sc hauf ler, Andrew Morton, linuxppc-dev, David S. Miller,
linux-arm-kernel
On Fri, Jul 8, 2016 at 11:17 PM, <Valdis.Kletnieks@vt.edu> wrote:
> Yeah, 'ping' dies with a similar traceback going to rawv6_setsockopt(),
> and 'trinity' dies a horrid death during initialization because it creates
> some sctp sockets to fool around with. The problem in all these cases is that
> setsockopt uses copy_from_user() to pull in the option value, and the allocation
> isn't tagged with USERCOPY to whitelist it.
Just a note to clear up confusion: this series doesn't include the
whitelist protection, so this appears to be either bugs in the slub
checker or bugs in the code using the cfq_io_cq cache. I suspect the
former. :)
-Kees
--
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support
2016-07-08 20:48 ` Kees Cook
` (6 preceding siblings ...)
[not found] ` <57809299.84b3370a.5390c.ffff9e58SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2016-07-11 6:08 ` Joonsoo Kim
7 siblings, 0 replies; 22+ messages in thread
From: Joonsoo Kim @ 2016-07-11 6:08 UTC (permalink / raw)
To: Kees Cook
Cc: Christoph Lameter, Michael Ellerman, kernel-hardening, Jan Kara,
Catalin Marinas, Will Deacon, Linux-MM, sparclinux, linux-ia64,
Andrea Arcangeli, linux-arch, x86, Russell King, PaX Team,
Borislav Petkov, Mathias Krause, Fenghua Yu, Rik van Riel,
David Rientjes, Tony Luck, Andy Lutomirski, Dmitry Vyukov,
Laura Abbott, Brad Spengler, Ard Biesheuvel, LKML, Pekka Enberg,
Case y Schauf ler, Andrew Morton, linuxppc-dev, David S. Miller,
linux-arm-kernel
On Fri, Jul 08, 2016 at 04:48:38PM -0400, Kees Cook wrote:
> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook <keescook@chromium.org> wrote:
> > On Fri, Jul 8, 2016 at 12:20 PM, Christoph Lameter <cl@linux.com> wrote:
> >> On Fri, 8 Jul 2016, Kees Cook wrote:
> >>
> >>> Is check_valid_pointer() making sure the pointer is within the usable
> >>> size? It seemed like it was checking that it was within the slub
> >>> object (checks against s->size, wants it above base after moving
> >>> pointer to include redzone, etc).
> >>
> >> check_valid_pointer verifies that a pointer is pointing to the start of an
> >> object. It is used to verify the internal points that SLUB used and
> >> should not be modified to do anything different.
> >
> > Yup, no worries -- I won't touch it. :) I just wanted to verify my
> > understanding.
> >
> > And after playing a bit more, I see that the only thing to the left is
> > padding and redzone. SLUB layout, from what I saw:
> >
> > offset: what's there
> > -------
> > start: padding, redzone
> > red_left_pad: object itself
> > inuse: rest of metadata
> > size: start of next slub object
> >
> > (and object_size == inuse - red_left_pad)
> >
> > i.e. a pointer must be between red_left_pad and inuse, which is the
> > same as pointer - ref_left_pad being less than object_size.
> >
> > So, as found already, the position in the usercopy check needs to be
> > bumped down by red_left_pad, which is what Michael's fix does, so I'll
> > include it in the next version.
>
> Actually, after some offline chats, I think this is better, since it
> makes sure the ptr doesn't end up somewhere weird before we start the
> calculations. This leaves the pointer as-is, but explicitly handles
> the redzone on the offset instead, with no wrapping, etc:
>
> /* Find offset within object. */
> offset = (ptr - page_address(page)) % s->size;
>
> + /* Adjust for redzone and reject if within the redzone. */
> + if (s->flags & SLAB_RED_ZONE) {
> + if (offset < s->red_left_pad)
> + return s->name;
> + offset -= s->red_left_pad;
> + }
> +
> /* Allow address range falling entirely within object size. */
> if (offset <= s->object_size && n <= s->object_size - offset)
> return NULL;
>
As Christoph saids, please use slab_ksize() rather than
s->object_size.
Otherwise, looks good to me.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread