* [kernel-hardening] [PATCH] lkdtm: add bad USER_DS test @ 2017-03-23 20:34 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-23 20:34 UTC (permalink / raw) To: Thomas Garnier Cc: Martin Schwidefsky, Heiko Carstens, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm.h | 1 + drivers/misc/lkdtm_bugs.c | 20 ++++++++++++++++++++ drivers/misc/lkdtm_core.c | 1 + 3 files changed, 22 insertions(+) diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 67d27be60405..3b4976396ec4 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -27,6 +27,7 @@ void lkdtm_REFCOUNT_ZERO_SUB(void); void lkdtm_REFCOUNT_ZERO_ADD(void); void lkdtm_CORRUPT_LIST_ADD(void); void lkdtm_CORRUPT_LIST_DEL(void); +void lkdtm_CORRUPT_USER_DS(void); /* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index e3f4cd8876b5..4906e53a6df3 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -8,6 +8,7 @@ #include <linux/list.h> #include <linux/refcount.h> #include <linux/sched.h> +#include <linux/uaccess.h> struct lkdtm_list { struct list_head node; @@ -279,3 +280,22 @@ void lkdtm_CORRUPT_LIST_DEL(void) else pr_err("list_del() corruption not detected!\n"); } + +void lkdtm_CORRUPT_USER_DS(void) +{ + /* + * Test that USER_DS has been set correctly on exiting a syscall. + * Since setting this higher than USER_DS (TASK_SIZE) would introduce + * an exploitable condition, we lower it instead, since that should + * not create as large a problem on an unprotected system. + */ + mm_segment_t lowfs; +#ifdef MAKE_MM_SEG + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); +#else + lowfs = TASK_SIZE - PAGE_SIZE; +#endif + + pr_info("setting bad task size limit\n"); + set_fs(lowfs); +} diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index b9a4cd4a9b68..42d2b8e31e6b 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -199,6 +199,7 @@ struct crashtype crashtypes[] = { CRASHTYPE(OVERFLOW), CRASHTYPE(CORRUPT_LIST_ADD), CRASHTYPE(CORRUPT_LIST_DEL), + CRASHTYPE(CORRUPT_USER_DS), CRASHTYPE(CORRUPT_STACK), CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), CRASHTYPE(OVERWRITE_ALLOCATION), -- 2.7.4 -- Kees Cook Pixel Security ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-23 20:34 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-23 20:34 UTC (permalink / raw) To: Thomas Garnier Cc: Martin Schwidefsky, Heiko Carstens, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm.h | 1 + drivers/misc/lkdtm_bugs.c | 20 ++++++++++++++++++++ drivers/misc/lkdtm_core.c | 1 + 3 files changed, 22 insertions(+) diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 67d27be60405..3b4976396ec4 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -27,6 +27,7 @@ void lkdtm_REFCOUNT_ZERO_SUB(void); void lkdtm_REFCOUNT_ZERO_ADD(void); void lkdtm_CORRUPT_LIST_ADD(void); void lkdtm_CORRUPT_LIST_DEL(void); +void lkdtm_CORRUPT_USER_DS(void); /* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index e3f4cd8876b5..4906e53a6df3 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -8,6 +8,7 @@ #include <linux/list.h> #include <linux/refcount.h> #include <linux/sched.h> +#include <linux/uaccess.h> struct lkdtm_list { struct list_head node; @@ -279,3 +280,22 @@ void lkdtm_CORRUPT_LIST_DEL(void) else pr_err("list_del() corruption not detected!\n"); } + +void lkdtm_CORRUPT_USER_DS(void) +{ + /* + * Test that USER_DS has been set correctly on exiting a syscall. + * Since setting this higher than USER_DS (TASK_SIZE) would introduce + * an exploitable condition, we lower it instead, since that should + * not create as large a problem on an unprotected system. + */ + mm_segment_t lowfs; +#ifdef MAKE_MM_SEG + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); +#else + lowfs = TASK_SIZE - PAGE_SIZE; +#endif + + pr_info("setting bad task size limit\n"); + set_fs(lowfs); +} diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index b9a4cd4a9b68..42d2b8e31e6b 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -199,6 +199,7 @@ struct crashtype crashtypes[] = { CRASHTYPE(OVERFLOW), CRASHTYPE(CORRUPT_LIST_ADD), CRASHTYPE(CORRUPT_LIST_DEL), + CRASHTYPE(CORRUPT_USER_DS), CRASHTYPE(CORRUPT_STACK), CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), CRASHTYPE(OVERWRITE_ALLOCATION), -- 2.7.4 -- Kees Cook Pixel Security ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-23 20:34 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-23 20:34 UTC (permalink / raw) To: linux-arm-kernel This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm.h | 1 + drivers/misc/lkdtm_bugs.c | 20 ++++++++++++++++++++ drivers/misc/lkdtm_core.c | 1 + 3 files changed, 22 insertions(+) diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 67d27be60405..3b4976396ec4 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -27,6 +27,7 @@ void lkdtm_REFCOUNT_ZERO_SUB(void); void lkdtm_REFCOUNT_ZERO_ADD(void); void lkdtm_CORRUPT_LIST_ADD(void); void lkdtm_CORRUPT_LIST_DEL(void); +void lkdtm_CORRUPT_USER_DS(void); /* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index e3f4cd8876b5..4906e53a6df3 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -8,6 +8,7 @@ #include <linux/list.h> #include <linux/refcount.h> #include <linux/sched.h> +#include <linux/uaccess.h> struct lkdtm_list { struct list_head node; @@ -279,3 +280,22 @@ void lkdtm_CORRUPT_LIST_DEL(void) else pr_err("list_del() corruption not detected!\n"); } + +void lkdtm_CORRUPT_USER_DS(void) +{ + /* + * Test that USER_DS has been set correctly on exiting a syscall. + * Since setting this higher than USER_DS (TASK_SIZE) would introduce + * an exploitable condition, we lower it instead, since that should + * not create as large a problem on an unprotected system. + */ + mm_segment_t lowfs; +#ifdef MAKE_MM_SEG + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); +#else + lowfs = TASK_SIZE - PAGE_SIZE; +#endif + + pr_info("setting bad task size limit\n"); + set_fs(lowfs); +} diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index b9a4cd4a9b68..42d2b8e31e6b 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -199,6 +199,7 @@ struct crashtype crashtypes[] = { CRASHTYPE(OVERFLOW), CRASHTYPE(CORRUPT_LIST_ADD), CRASHTYPE(CORRUPT_LIST_DEL), + CRASHTYPE(CORRUPT_USER_DS), CRASHTYPE(CORRUPT_STACK), CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), CRASHTYPE(OVERWRITE_ALLOCATION), -- 2.7.4 -- Kees Cook Pixel Security ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-23 20:34 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-23 20:34 UTC (permalink / raw) To: Thomas Garnier Cc: Martin Schwidefsky, Heiko Carstens, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- drivers/misc/lkdtm.h | 1 + drivers/misc/lkdtm_bugs.c | 20 ++++++++++++++++++++ drivers/misc/lkdtm_core.c | 1 + 3 files changed, 22 insertions(+) diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index 67d27be60405..3b4976396ec4 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -27,6 +27,7 @@ void lkdtm_REFCOUNT_ZERO_SUB(void); void lkdtm_REFCOUNT_ZERO_ADD(void); void lkdtm_CORRUPT_LIST_ADD(void); void lkdtm_CORRUPT_LIST_DEL(void); +void lkdtm_CORRUPT_USER_DS(void); /* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index e3f4cd8876b5..4906e53a6df3 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -8,6 +8,7 @@ #include <linux/list.h> #include <linux/refcount.h> #include <linux/sched.h> +#include <linux/uaccess.h> struct lkdtm_list { struct list_head node; @@ -279,3 +280,22 @@ void lkdtm_CORRUPT_LIST_DEL(void) else pr_err("list_del() corruption not detected!\n"); } + +void lkdtm_CORRUPT_USER_DS(void) +{ + /* + * Test that USER_DS has been set correctly on exiting a syscall. + * Since setting this higher than USER_DS (TASK_SIZE) would introduce + * an exploitable condition, we lower it instead, since that should + * not create as large a problem on an unprotected system. + */ + mm_segment_t lowfs; +#ifdef MAKE_MM_SEG + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); +#else + lowfs = TASK_SIZE - PAGE_SIZE; +#endif + + pr_info("setting bad task size limit\n"); + set_fs(lowfs); +} diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index b9a4cd4a9b68..42d2b8e31e6b 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -199,6 +199,7 @@ struct crashtype crashtypes[] = { CRASHTYPE(OVERFLOW), CRASHTYPE(CORRUPT_LIST_ADD), CRASHTYPE(CORRUPT_LIST_DEL), + CRASHTYPE(CORRUPT_USER_DS), CRASHTYPE(CORRUPT_STACK), CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE), CRASHTYPE(OVERWRITE_ALLOCATION), -- 2.7.4 -- Kees Cook Pixel Security ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [kernel-hardening] Re: [PATCH] lkdtm: add bad USER_DS test 2017-03-23 20:34 ` Kees Cook (?) (?) @ 2017-03-24 8:14 ` Heiko Carstens -1 siblings, 0 replies; 24+ messages in thread From: Heiko Carstens @ 2017-03-24 8:14 UTC (permalink / raw) To: Kees Cook Cc: Thomas Garnier, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: > This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return > still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. > > Signed-off-by: Kees Cook <keescook@chromium.org> ... > +void lkdtm_CORRUPT_USER_DS(void) > +{ > + /* > + * Test that USER_DS has been set correctly on exiting a syscall. > + * Since setting this higher than USER_DS (TASK_SIZE) would introduce > + * an exploitable condition, we lower it instead, since that should > + * not create as large a problem on an unprotected system. > + */ > + mm_segment_t lowfs; > +#ifdef MAKE_MM_SEG > + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); > +#else > + lowfs = TASK_SIZE - PAGE_SIZE; > +#endif > + > + pr_info("setting bad task size limit\n"); > + set_fs(lowfs); > +} This won't work on architectures where the set_fs() argument does not contain an address but an address space identifier. This is true e.g. for s390 and as far as I know also for sparc. On s390 we have complete distinct address spaces for kernel and user space that each start at address zero. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 8:14 ` Heiko Carstens 0 siblings, 0 replies; 24+ messages in thread From: Heiko Carstens @ 2017-03-24 8:14 UTC (permalink / raw) To: Kees Cook Cc: Thomas Garnier, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: > This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return > still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. > > Signed-off-by: Kees Cook <keescook@chromium.org> ... > +void lkdtm_CORRUPT_USER_DS(void) > +{ > + /* > + * Test that USER_DS has been set correctly on exiting a syscall. > + * Since setting this higher than USER_DS (TASK_SIZE) would introduce > + * an exploitable condition, we lower it instead, since that should > + * not create as large a problem on an unprotected system. > + */ > + mm_segment_t lowfs; > +#ifdef MAKE_MM_SEG > + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); > +#else > + lowfs = TASK_SIZE - PAGE_SIZE; > +#endif > + > + pr_info("setting bad task size limit\n"); > + set_fs(lowfs); > +} This won't work on architectures where the set_fs() argument does not contain an address but an address space identifier. This is true e.g. for s390 and as far as I know also for sparc. On s390 we have complete distinct address spaces for kernel and user space that each start at address zero. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 8:14 ` Heiko Carstens 0 siblings, 0 replies; 24+ messages in thread From: Heiko Carstens @ 2017-03-24 8:14 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: > This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return > still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. > > Signed-off-by: Kees Cook <keescook@chromium.org> ... > +void lkdtm_CORRUPT_USER_DS(void) > +{ > + /* > + * Test that USER_DS has been set correctly on exiting a syscall. > + * Since setting this higher than USER_DS (TASK_SIZE) would introduce > + * an exploitable condition, we lower it instead, since that should > + * not create as large a problem on an unprotected system. > + */ > + mm_segment_t lowfs; > +#ifdef MAKE_MM_SEG > + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); > +#else > + lowfs = TASK_SIZE - PAGE_SIZE; > +#endif > + > + pr_info("setting bad task size limit\n"); > + set_fs(lowfs); > +} This won't work on architectures where the set_fs() argument does not contain an address but an address space identifier. This is true e.g. for s390 and as far as I know also for sparc. On s390 we have complete distinct address spaces for kernel and user space that each start at address zero. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 8:14 ` Heiko Carstens 0 siblings, 0 replies; 24+ messages in thread From: Heiko Carstens @ 2017-03-24 8:14 UTC (permalink / raw) To: Kees Cook Cc: Thomas Garnier, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: > This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return > still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. > > Signed-off-by: Kees Cook <keescook@chromium.org> ... > +void lkdtm_CORRUPT_USER_DS(void) > +{ > + /* > + * Test that USER_DS has been set correctly on exiting a syscall. > + * Since setting this higher than USER_DS (TASK_SIZE) would introduce > + * an exploitable condition, we lower it instead, since that should > + * not create as large a problem on an unprotected system. > + */ > + mm_segment_t lowfs; > +#ifdef MAKE_MM_SEG > + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); > +#else > + lowfs = TASK_SIZE - PAGE_SIZE; > +#endif > + > + pr_info("setting bad task size limit\n"); > + set_fs(lowfs); > +} This won't work on architectures where the set_fs() argument does not contain an address but an address space identifier. This is true e.g. for s390 and as far as I know also for sparc. On s390 we have complete distinct address spaces for kernel and user space that each start at address zero. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [kernel-hardening] Re: [PATCH] lkdtm: add bad USER_DS test 2017-03-24 8:14 ` Heiko Carstens (?) (?) @ 2017-03-24 15:17 ` Thomas Garnier -1 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 15:17 UTC (permalink / raw) To: Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> > > ... > >> +void lkdtm_CORRUPT_USER_DS(void) >> +{ >> + /* >> + * Test that USER_DS has been set correctly on exiting a syscall. >> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >> + * an exploitable condition, we lower it instead, since that should >> + * not create as large a problem on an unprotected system. >> + */ >> + mm_segment_t lowfs; >> +#ifdef MAKE_MM_SEG >> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >> +#else >> + lowfs = TASK_SIZE - PAGE_SIZE; >> +#endif >> + >> + pr_info("setting bad task size limit\n"); >> + set_fs(lowfs); >> +} > > This won't work on architectures where the set_fs() argument does not > contain an address but an address space identifier. This is true e.g. for > s390 and as far as I know also for sparc. > On s390 we have complete distinct address spaces for kernel and user space > that each start at address zero. > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we can just do a set_fs(KERNEL_DS) for the others. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:17 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 15:17 UTC (permalink / raw) To: Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Christian Borntraeger, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> > > ... > >> +void lkdtm_CORRUPT_USER_DS(void) >> +{ >> + /* >> + * Test that USER_DS has been set correctly on exiting a syscall. >> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >> + * an exploitable condition, we lower it instead, since that should >> + * not create as large a problem on an unprotected system. >> + */ >> + mm_segment_t lowfs; >> +#ifdef MAKE_MM_SEG >> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >> +#else >> + lowfs = TASK_SIZE - PAGE_SIZE; >> +#endif >> + >> + pr_info("setting bad task size limit\n"); >> + set_fs(lowfs); >> +} > > This won't work on architectures where the set_fs() argument does not > contain an address but an address space identifier. This is true e.g. for > s390 and as far as I know also for sparc. > On s390 we have complete distinct address spaces for kernel and user space > that each start at address zero. > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we can just do a set_fs(KERNEL_DS) for the others. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:17 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 15:17 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> > > ... > >> +void lkdtm_CORRUPT_USER_DS(void) >> +{ >> + /* >> + * Test that USER_DS has been set correctly on exiting a syscall. >> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >> + * an exploitable condition, we lower it instead, since that should >> + * not create as large a problem on an unprotected system. >> + */ >> + mm_segment_t lowfs; >> +#ifdef MAKE_MM_SEG >> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >> +#else >> + lowfs = TASK_SIZE - PAGE_SIZE; >> +#endif >> + >> + pr_info("setting bad task size limit\n"); >> + set_fs(lowfs); >> +} > > This won't work on architectures where the set_fs() argument does not > contain an address but an address space identifier. This is true e.g. for > s390 and as far as I know also for sparc. > On s390 we have complete distinct address spaces for kernel and user space > that each start at address zero. > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we can just do a set_fs(KERNEL_DS) for the others. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:17 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 15:17 UTC (permalink / raw) To: Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens <heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> wrote: > On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >> >> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > > ... > >> +void lkdtm_CORRUPT_USER_DS(void) >> +{ >> + /* >> + * Test that USER_DS has been set correctly on exiting a syscall. >> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >> + * an exploitable condition, we lower it instead, since that should >> + * not create as large a problem on an unprotected system. >> + */ >> + mm_segment_t lowfs; >> +#ifdef MAKE_MM_SEG >> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >> +#else >> + lowfs = TASK_SIZE - PAGE_SIZE; >> +#endif >> + >> + pr_info("setting bad task size limit\n"); >> + set_fs(lowfs); >> +} > > This won't work on architectures where the set_fs() argument does not > contain an address but an address space identifier. This is true e.g. for > s390 and as far as I know also for sparc. > On s390 we have complete distinct address spaces for kernel and user space > that each start at address zero. > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we can just do a set_fs(KERNEL_DS) for the others. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [kernel-hardening] Re: [PATCH] lkdtm: add bad USER_DS test 2017-03-24 15:17 ` Thomas Garnier (?) (?) @ 2017-03-24 15:24 ` Christian Borntraeger -1 siblings, 0 replies; 24+ messages in thread From: Christian Borntraeger @ 2017-03-24 15:24 UTC (permalink / raw) To: Thomas Garnier, Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On 03/24/2017 04:17 PM, Thomas Garnier wrote: > On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: >> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> ... >> >>> +void lkdtm_CORRUPT_USER_DS(void) >>> +{ >>> + /* >>> + * Test that USER_DS has been set correctly on exiting a syscall. >>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>> + * an exploitable condition, we lower it instead, since that should >>> + * not create as large a problem on an unprotected system. >>> + */ >>> + mm_segment_t lowfs; >>> +#ifdef MAKE_MM_SEG >>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>> +#else >>> + lowfs = TASK_SIZE - PAGE_SIZE; >>> +#endif >>> + >>> + pr_info("setting bad task size limit\n"); >>> + set_fs(lowfs); >>> +} >> >> This won't work on architectures where the set_fs() argument does not >> contain an address but an address space identifier. This is true e.g. for >> s390 and as far as I know also for sparc. >> On s390 we have complete distinct address spaces for kernel and user space >> that each start at address zero. >> > > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we > can just do a set_fs(KERNEL_DS) for the others. that would enable the test, but it would also mean that lkdtm can be used by a program to escalate its rights. I think that is the reason why Kees did this lowfs things. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:24 ` Christian Borntraeger 0 siblings, 0 replies; 24+ messages in thread From: Christian Borntraeger @ 2017-03-24 15:24 UTC (permalink / raw) To: Thomas Garnier, Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On 03/24/2017 04:17 PM, Thomas Garnier wrote: > On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: >> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> ... >> >>> +void lkdtm_CORRUPT_USER_DS(void) >>> +{ >>> + /* >>> + * Test that USER_DS has been set correctly on exiting a syscall. >>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>> + * an exploitable condition, we lower it instead, since that should >>> + * not create as large a problem on an unprotected system. >>> + */ >>> + mm_segment_t lowfs; >>> +#ifdef MAKE_MM_SEG >>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>> +#else >>> + lowfs = TASK_SIZE - PAGE_SIZE; >>> +#endif >>> + >>> + pr_info("setting bad task size limit\n"); >>> + set_fs(lowfs); >>> +} >> >> This won't work on architectures where the set_fs() argument does not >> contain an address but an address space identifier. This is true e.g. for >> s390 and as far as I know also for sparc. >> On s390 we have complete distinct address spaces for kernel and user space >> that each start at address zero. >> > > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we > can just do a set_fs(KERNEL_DS) for the others. that would enable the test, but it would also mean that lkdtm can be used by a program to escalate its rights. I think that is the reason why Kees did this lowfs things. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:24 ` Christian Borntraeger 0 siblings, 0 replies; 24+ messages in thread From: Christian Borntraeger @ 2017-03-24 15:24 UTC (permalink / raw) To: linux-arm-kernel On 03/24/2017 04:17 PM, Thomas Garnier wrote: > On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: >> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> ... >> >>> +void lkdtm_CORRUPT_USER_DS(void) >>> +{ >>> + /* >>> + * Test that USER_DS has been set correctly on exiting a syscall. >>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>> + * an exploitable condition, we lower it instead, since that should >>> + * not create as large a problem on an unprotected system. >>> + */ >>> + mm_segment_t lowfs; >>> +#ifdef MAKE_MM_SEG >>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>> +#else >>> + lowfs = TASK_SIZE - PAGE_SIZE; >>> +#endif >>> + >>> + pr_info("setting bad task size limit\n"); >>> + set_fs(lowfs); >>> +} >> >> This won't work on architectures where the set_fs() argument does not >> contain an address but an address space identifier. This is true e.g. for >> s390 and as far as I know also for sparc. >> On s390 we have complete distinct address spaces for kernel and user space >> that each start at address zero. >> > > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we > can just do a set_fs(KERNEL_DS) for the others. that would enable the test, but it would also mean that lkdtm can be used by a program to escalate its rights. I think that is the reason why Kees did this lowfs things. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 15:24 ` Christian Borntraeger 0 siblings, 0 replies; 24+ messages in thread From: Christian Borntraeger @ 2017-03-24 15:24 UTC (permalink / raw) To: Thomas Garnier, Heiko Carstens Cc: Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov On 03/24/2017 04:17 PM, Thomas Garnier wrote: > On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: >> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> ... >> >>> +void lkdtm_CORRUPT_USER_DS(void) >>> +{ >>> + /* >>> + * Test that USER_DS has been set correctly on exiting a syscall. >>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>> + * an exploitable condition, we lower it instead, since that should >>> + * not create as large a problem on an unprotected system. >>> + */ >>> + mm_segment_t lowfs; >>> +#ifdef MAKE_MM_SEG >>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>> +#else >>> + lowfs = TASK_SIZE - PAGE_SIZE; >>> +#endif >>> + >>> + pr_info("setting bad task size limit\n"); >>> + set_fs(lowfs); >>> +} >> >> This won't work on architectures where the set_fs() argument does not >> contain an address but an address space identifier. This is true e.g. for >> s390 and as far as I know also for sparc. >> On s390 we have complete distinct address spaces for kernel and user space >> that each start at address zero. >> > > The patch that enforce USER_DS is disabled on s390 anyway. I guess, we > can just do a set_fs(KERNEL_DS) for the others. that would enable the test, but it would also mean that lkdtm can be used by a program to escalate its rights. I think that is the reason why Kees did this lowfs things. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [kernel-hardening] Re: [PATCH] lkdtm: add bad USER_DS test 2017-03-24 15:24 ` Christian Borntraeger (?) (?) @ 2017-03-24 16:11 ` Thomas Garnier -1 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 16:11 UTC (permalink / raw) To: Christian Borntraeger Cc: Heiko Carstens, Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. > I see, I need to look at how lkdtm works exactly. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 16:11 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 16:11 UTC (permalink / raw) To: Christian Borntraeger Cc: Heiko Carstens, Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. > I see, I need to look at how lkdtm works exactly. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 16:11 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 16:11 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. > I see, I need to look at how lkdtm works exactly. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 16:11 ` Thomas Garnier 0 siblings, 0 replies; 24+ messages in thread From: Thomas Garnier @ 2017-03-24 16:11 UTC (permalink / raw) To: Christian Borntraeger Cc: Heiko Carstens, Kees Cook, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. > I see, I need to look at how lkdtm works exactly. -- Thomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [kernel-hardening] Re: [PATCH] lkdtm: add bad USER_DS test 2017-03-24 15:24 ` Christian Borntraeger (?) (?) @ 2017-03-24 17:46 ` Kees Cook -1 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-24 17:46 UTC (permalink / raw) To: Christian Borntraeger Cc: Thomas Garnier, Heiko Carstens, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. Yeah, but it seems like getting this right for all architectures isn't sane. I'm going to change it to use KERNEL_DS but also post a SIGKILL to the process. That way it'll still trigger the syscall return checking, but will be unable to continue running with a potentially uncaught KERNEL_DS. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 17:46 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-24 17:46 UTC (permalink / raw) To: Christian Borntraeger Cc: Thomas Garnier, Heiko Carstens, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst, Kirill A . Shutemov, Russell King, Will Deacon, Catalin Marinas, Mark Rutland, James Morse, linux-s390@vger.kernel.org, LKML, Linux API, x86@kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. Yeah, but it seems like getting this right for all architectures isn't sane. I'm going to change it to use KERNEL_DS but also post a SIGKILL to the process. That way it'll still trigger the syscall return checking, but will be unable to continue running with a potentially uncaught KERNEL_DS. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 17:46 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-24 17:46 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. Yeah, but it seems like getting this right for all architectures isn't sane. I'm going to change it to use KERNEL_DS but also post a SIGKILL to the process. That way it'll still trigger the syscall return checking, but will be unable to continue running with a potentially uncaught KERNEL_DS. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] lkdtm: add bad USER_DS test @ 2017-03-24 17:46 ` Kees Cook 0 siblings, 0 replies; 24+ messages in thread From: Kees Cook @ 2017-03-24 17:46 UTC (permalink / raw) To: Christian Borntraeger Cc: Thomas Garnier, Heiko Carstens, Martin Schwidefsky, David Howells, Arnd Bergmann, Dave Hansen, Al Viro, Thomas Gleixner, René Nyffenegger, Andrew Morton, Paul E . McKenney, Ingo Molnar, Oleg Nesterov, Pavel Tikhomirov, Stephen Smalley, Ingo Molnar, H . Peter Anvin, Andy Lutomirski, Paolo Bonzini, Rik van Riel, Josh Poimboeuf, Borislav Petkov, Brian Gerst On Fri, Mar 24, 2017 at 8:24 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 03/24/2017 04:17 PM, Thomas Garnier wrote: >> On Fri, Mar 24, 2017 at 1:14 AM, Heiko Carstens >> <heiko.carstens@de.ibm.com> wrote: >>> On Thu, Mar 23, 2017 at 01:34:19PM -0700, Kees Cook wrote: >>>> This adds CORRUPT_USER_DS to check that the get_fs() test on syscall return >>>> still sees USER_DS during the new VERIFY_PRE_USERMODE_STATE checks. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> >>> ... >>> >>>> +void lkdtm_CORRUPT_USER_DS(void) >>>> +{ >>>> + /* >>>> + * Test that USER_DS has been set correctly on exiting a syscall. >>>> + * Since setting this higher than USER_DS (TASK_SIZE) would introduce >>>> + * an exploitable condition, we lower it instead, since that should >>>> + * not create as large a problem on an unprotected system. >>>> + */ >>>> + mm_segment_t lowfs; >>>> +#ifdef MAKE_MM_SEG >>>> + lowfs = MAKE_MM_SEG(TASK_SIZE - PAGE_SIZE); >>>> +#else >>>> + lowfs = TASK_SIZE - PAGE_SIZE; >>>> +#endif >>>> + >>>> + pr_info("setting bad task size limit\n"); >>>> + set_fs(lowfs); >>>> +} >>> >>> This won't work on architectures where the set_fs() argument does not >>> contain an address but an address space identifier. This is true e.g. for >>> s390 and as far as I know also for sparc. >>> On s390 we have complete distinct address spaces for kernel and user space >>> that each start at address zero. >>> >> >> The patch that enforce USER_DS is disabled on s390 anyway. I guess, we >> can just do a set_fs(KERNEL_DS) for the others. > > that would enable the test, but it would also mean that lkdtm can be used by > a program to escalate its rights. I think that is the reason why Kees did this > lowfs things. Yeah, but it seems like getting this right for all architectures isn't sane. I'm going to change it to use KERNEL_DS but also post a SIGKILL to the process. That way it'll still trigger the syscall return checking, but will be unable to continue running with a potentially uncaught KERNEL_DS. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-03-24 17:46 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-23 20:34 [kernel-hardening] [PATCH] lkdtm: add bad USER_DS test Kees Cook 2017-03-23 20:34 ` Kees Cook 2017-03-23 20:34 ` Kees Cook 2017-03-23 20:34 ` Kees Cook 2017-03-24 8:14 ` [kernel-hardening] " Heiko Carstens 2017-03-24 8:14 ` Heiko Carstens 2017-03-24 8:14 ` Heiko Carstens 2017-03-24 8:14 ` Heiko Carstens 2017-03-24 15:17 ` [kernel-hardening] " Thomas Garnier 2017-03-24 15:17 ` Thomas Garnier 2017-03-24 15:17 ` Thomas Garnier 2017-03-24 15:17 ` Thomas Garnier 2017-03-24 15:24 ` [kernel-hardening] " Christian Borntraeger 2017-03-24 15:24 ` Christian Borntraeger 2017-03-24 15:24 ` Christian Borntraeger 2017-03-24 15:24 ` Christian Borntraeger 2017-03-24 16:11 ` [kernel-hardening] " Thomas Garnier 2017-03-24 16:11 ` Thomas Garnier 2017-03-24 16:11 ` Thomas Garnier 2017-03-24 16:11 ` Thomas Garnier 2017-03-24 17:46 ` [kernel-hardening] " Kees Cook 2017-03-24 17:46 ` Kees Cook 2017-03-24 17:46 ` Kees Cook 2017-03-24 17:46 ` Kees Cook
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.