* [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption @ 2017-07-17 16:45 Alexander Popov 2017-07-17 16:57 ` Christopher Lameter 2017-07-17 17:54 ` Matthew Wilcox 0 siblings, 2 replies; 12+ messages in thread From: Alexander Popov @ 2017-07-17 16:45 UTC (permalink / raw) To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook, alex.popov Add an assertion similar to "fasttop" check in GNU C Library allocator: an object added to a singly linked freelist should not point to itself. That helps to detect some double free errors (e.g. CVE-2017-2636) without slub_debug and KASAN. Testing with hackbench doesn't show any noticeable performance penalty. Signed-off-by: Alexander Popov <alex.popov@linux.com> --- mm/slub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mm/slub.c b/mm/slub.c index 1d3f983..a106939b 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -261,6 +261,7 @@ static inline void *get_freepointer_safe(struct kmem_cache *s, void *object) static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp) { + BUG_ON(object == fp); /* naive detection of double free or corruption */ *(void **)(object + s->offset) = fp; } -- 2.7.4 -- 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 related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 16:45 [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption Alexander Popov @ 2017-07-17 16:57 ` Christopher Lameter 2017-07-17 17:54 ` Matthew Wilcox 1 sibling, 0 replies; 12+ messages in thread From: Christopher Lameter @ 2017-07-17 16:57 UTC (permalink / raw) To: Alexander Popov Cc: Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook On Mon, 17 Jul 2017, Alexander Popov wrote: > Add an assertion similar to "fasttop" check in GNU C Library allocator: > an object added to a singly linked freelist should not point to itself. > That helps to detect some double free errors (e.g. CVE-2017-2636) without > slub_debug and KASAN. Testing with hackbench doesn't show any noticeable > performance penalty. We are adding up "unnoticable performance penalties". This is used int both the critical allocation and free paths. Could this be VM_BUG_ON()? > > Signed-off-by: Alexander Popov <alex.popov@linux.com> > --- > mm/slub.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/slub.c b/mm/slub.c > index 1d3f983..a106939b 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -261,6 +261,7 @@ static inline void *get_freepointer_safe(struct kmem_cache *s, void *object) > > static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp) > { > + BUG_ON(object == fp); /* naive detection of double free or corruption */ > *(void **)(object + s->offset) = fp; > } > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 16:45 [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption Alexander Popov 2017-07-17 16:57 ` Christopher Lameter @ 2017-07-17 17:54 ` Matthew Wilcox 2017-07-17 18:04 ` Christopher Lameter 2017-07-17 18:23 ` Alexander Popov 1 sibling, 2 replies; 12+ messages in thread From: Matthew Wilcox @ 2017-07-17 17:54 UTC (permalink / raw) To: Alexander Popov Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: > Add an assertion similar to "fasttop" check in GNU C Library allocator: > an object added to a singly linked freelist should not point to itself. > That helps to detect some double free errors (e.g. CVE-2017-2636) without > slub_debug and KASAN. Testing with hackbench doesn't show any noticeable > performance penalty. > { > + BUG_ON(object == fp); /* naive detection of double free or corruption */ > *(void **)(object + s->offset) = fp; > } Is BUG() the best response to this situation? If it's a corruption, then yes, but if we spot a double-free, then surely we should WARN() and return without doing anything? -- 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 17:54 ` Matthew Wilcox @ 2017-07-17 18:04 ` Christopher Lameter 2017-07-17 19:01 ` Alexander Popov 2017-07-17 18:23 ` Alexander Popov 1 sibling, 1 reply; 12+ messages in thread From: Christopher Lameter @ 2017-07-17 18:04 UTC (permalink / raw) To: Matthew Wilcox Cc: Alexander Popov, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook On Mon, 17 Jul 2017, Matthew Wilcox wrote: > On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: > > Add an assertion similar to "fasttop" check in GNU C Library allocator: > > an object added to a singly linked freelist should not point to itself. > > That helps to detect some double free errors (e.g. CVE-2017-2636) without > > slub_debug and KASAN. Testing with hackbench doesn't show any noticeable > > performance penalty. > > > { > > + BUG_ON(object == fp); /* naive detection of double free or corruption */ > > *(void **)(object + s->offset) = fp; > > } > > Is BUG() the best response to this situation? If it's a corruption, then > yes, but if we spot a double-free, then surely we should WARN() and return > without doing anything? The double free debug checking already does the same thing in a more thourough way (this one only checks if the last free was the same address). So its duplicating a check that already exists. However, this one is always on. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 18:04 ` Christopher Lameter @ 2017-07-17 19:01 ` Alexander Popov 2017-07-17 19:11 ` Kees Cook 2017-07-18 14:57 ` Christopher Lameter 0 siblings, 2 replies; 12+ messages in thread From: Alexander Popov @ 2017-07-17 19:01 UTC (permalink / raw) To: Christopher Lameter, Matthew Wilcox Cc: Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook Hello Christopher, Thanks for your reply. On 17.07.2017 21:04, Christopher Lameter wrote: > On Mon, 17 Jul 2017, Matthew Wilcox wrote: > >> On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: >>> Add an assertion similar to "fasttop" check in GNU C Library allocator: >>> an object added to a singly linked freelist should not point to itself. >>> That helps to detect some double free errors (e.g. CVE-2017-2636) without >>> slub_debug and KASAN. Testing with hackbench doesn't show any noticeable >>> performance penalty. >> >>> { >>> + BUG_ON(object == fp); /* naive detection of double free or corruption */ >>> *(void **)(object + s->offset) = fp; >>> } >> >> Is BUG() the best response to this situation? If it's a corruption, then >> yes, but if we spot a double-free, then surely we should WARN() and return >> without doing anything? > > The double free debug checking already does the same thing in a more > thourough way (this one only checks if the last free was the same > address). So its duplicating a check that already exists. Yes, absolutely. Enabled slub_debug (or KASAN with its quarantine) can detect more double-free errors. But it introduces much bigger performance penalty and it's disabled by default. > However, this one is always on. Yes, I would propose to have this relatively cheap check enabled by default. I think it will block a good share of double-free errors. Currently it's really easy to turn such a double-free into use-after-free and exploit it, since, as I wrote, next two kmalloc() calls return the same address. So we could make exploiting harder for a relatively low price. Christopher, if I change BUG_ON() to VM_BUG_ON(), it will be disabled by default again, right? Best regards, Alexander -- 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 19:01 ` Alexander Popov @ 2017-07-17 19:11 ` Kees Cook 2017-07-18 19:56 ` Alexander Popov 2017-07-18 14:57 ` Christopher Lameter 1 sibling, 1 reply; 12+ messages in thread From: Kees Cook @ 2017-07-17 19:11 UTC (permalink / raw) To: Alexander Popov Cc: Christopher Lameter, Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, Linux-MM, LKML, kernel-hardening On Mon, Jul 17, 2017 at 12:01 PM, Alexander Popov <alex.popov@linux.com> wrote: > Hello Christopher, > > Thanks for your reply. > > On 17.07.2017 21:04, Christopher Lameter wrote: >> On Mon, 17 Jul 2017, Matthew Wilcox wrote: >> >>> On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: >>>> Add an assertion similar to "fasttop" check in GNU C Library allocator: >>>> an object added to a singly linked freelist should not point to itself. >>>> That helps to detect some double free errors (e.g. CVE-2017-2636) without >>>> slub_debug and KASAN. Testing with hackbench doesn't show any noticeable >>>> performance penalty. >>> >>>> { >>>> + BUG_ON(object == fp); /* naive detection of double free or corruption */ >>>> *(void **)(object + s->offset) = fp; >>>> } >>> >>> Is BUG() the best response to this situation? If it's a corruption, then >>> yes, but if we spot a double-free, then surely we should WARN() and return >>> without doing anything? >> >> The double free debug checking already does the same thing in a more >> thourough way (this one only checks if the last free was the same >> address). So its duplicating a check that already exists. > > Yes, absolutely. Enabled slub_debug (or KASAN with its quarantine) can detect > more double-free errors. But it introduces much bigger performance penalty and > it's disabled by default. > >> However, this one is always on. > > Yes, I would propose to have this relatively cheap check enabled by default. I > think it will block a good share of double-free errors. Currently it's really > easy to turn such a double-free into use-after-free and exploit it, since, as I > wrote, next two kmalloc() calls return the same address. So we could make > exploiting harder for a relatively low price. > > Christopher, if I change BUG_ON() to VM_BUG_ON(), it will be disabled by default > again, right? Let's merge this with the proposed CONFIG_FREELIST_HARDENED, then the performance change is behind a config, and we gain the rest of the freelist protections at the same time: http://www.openwall.com/lists/kernel-hardening/2017/07/06/1 -Kees -- Kees Cook Pixel 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 19:11 ` Kees Cook @ 2017-07-18 19:56 ` Alexander Popov 2017-07-18 20:04 ` Kees Cook 0 siblings, 1 reply; 12+ messages in thread From: Alexander Popov @ 2017-07-18 19:56 UTC (permalink / raw) To: Kees Cook Cc: Christopher Lameter, Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, Linux-MM, LKML, kernel-hardening On 17.07.2017 22:11, Kees Cook wrote: > On Mon, Jul 17, 2017 at 12:01 PM, Alexander Popov <alex.popov@linux.com> wrote: >> Hello Christopher, >> >> Thanks for your reply. >> >> On 17.07.2017 21:04, Christopher Lameter wrote: >>> On Mon, 17 Jul 2017, Matthew Wilcox wrote: >>> >>>> On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: >>>>> Add an assertion similar to "fasttop" check in GNU C Library allocator: >>>>> an object added to a singly linked freelist should not point to itself. >>>>> That helps to detect some double free errors (e.g. CVE-2017-2636) without >>>>> slub_debug and KASAN. Testing with hackbench doesn't show any noticeable >>>>> performance penalty. >>>> >>>>> { >>>>> + BUG_ON(object == fp); /* naive detection of double free or corruption */ >>>>> *(void **)(object + s->offset) = fp; >>>>> } >>>> >>>> Is BUG() the best response to this situation? If it's a corruption, then >>>> yes, but if we spot a double-free, then surely we should WARN() and return >>>> without doing anything? >>> >>> The double free debug checking already does the same thing in a more >>> thourough way (this one only checks if the last free was the same >>> address). So its duplicating a check that already exists. >> >> Yes, absolutely. Enabled slub_debug (or KASAN with its quarantine) can detect >> more double-free errors. But it introduces much bigger performance penalty and >> it's disabled by default. >> >>> However, this one is always on. >> >> Yes, I would propose to have this relatively cheap check enabled by default. I >> think it will block a good share of double-free errors. Currently it's really >> easy to turn such a double-free into use-after-free and exploit it, since, as I >> wrote, next two kmalloc() calls return the same address. So we could make >> exploiting harder for a relatively low price. >> >> Christopher, if I change BUG_ON() to VM_BUG_ON(), it will be disabled by default >> again, right? > > Let's merge this with the proposed CONFIG_FREELIST_HARDENED, then the > performance change is behind a config, and we gain the rest of the > freelist protections at the same time: > > http://www.openwall.com/lists/kernel-hardening/2017/07/06/1 Hello Kees, If I change BUG_ON() to VM_BUG_ON(), this check will work at least on Fedora since it has CONFIG_DEBUG_VM enabled. Debian based distros have this option disabled. Do you like that more than having this check under CONFIG_FREELIST_HARDENED? If you insist on putting this check under CONFIG_FREELIST_HARDENED, should I rebase onto your patch and send again? Best regards, Alexander -- 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-18 19:56 ` Alexander Popov @ 2017-07-18 20:04 ` Kees Cook 2017-07-19 8:38 ` Alexander Popov 2017-07-19 14:02 ` Christopher Lameter 0 siblings, 2 replies; 12+ messages in thread From: Kees Cook @ 2017-07-18 20:04 UTC (permalink / raw) To: Alexander Popov Cc: Christopher Lameter, Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, Linux-MM, LKML, kernel-hardening On Tue, Jul 18, 2017 at 12:56 PM, Alexander Popov <alex.popov@linux.com> wrote: > On 17.07.2017 22:11, Kees Cook wrote: >> On Mon, Jul 17, 2017 at 12:01 PM, Alexander Popov <alex.popov@linux.com> wrote: >>> Hello Christopher, >>> >>> Thanks for your reply. >>> >>> On 17.07.2017 21:04, Christopher Lameter wrote: >>>> On Mon, 17 Jul 2017, Matthew Wilcox wrote: >>>> >>>>> On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: >>>>>> Add an assertion similar to "fasttop" check in GNU C Library allocator: >>>>>> an object added to a singly linked freelist should not point to itself. >>>>>> That helps to detect some double free errors (e.g. CVE-2017-2636) without >>>>>> slub_debug and KASAN. Testing with hackbench doesn't show any noticeable >>>>>> performance penalty. >>>>> >>>>>> { >>>>>> + BUG_ON(object == fp); /* naive detection of double free or corruption */ >>>>>> *(void **)(object + s->offset) = fp; >>>>>> } >>>>> >>>>> Is BUG() the best response to this situation? If it's a corruption, then >>>>> yes, but if we spot a double-free, then surely we should WARN() and return >>>>> without doing anything? >>>> >>>> The double free debug checking already does the same thing in a more >>>> thourough way (this one only checks if the last free was the same >>>> address). So its duplicating a check that already exists. >>> >>> Yes, absolutely. Enabled slub_debug (or KASAN with its quarantine) can detect >>> more double-free errors. But it introduces much bigger performance penalty and >>> it's disabled by default. >>> >>>> However, this one is always on. >>> >>> Yes, I would propose to have this relatively cheap check enabled by default. I >>> think it will block a good share of double-free errors. Currently it's really >>> easy to turn such a double-free into use-after-free and exploit it, since, as I >>> wrote, next two kmalloc() calls return the same address. So we could make >>> exploiting harder for a relatively low price. >>> >>> Christopher, if I change BUG_ON() to VM_BUG_ON(), it will be disabled by default >>> again, right? >> >> Let's merge this with the proposed CONFIG_FREELIST_HARDENED, then the >> performance change is behind a config, and we gain the rest of the >> freelist protections at the same time: >> >> http://www.openwall.com/lists/kernel-hardening/2017/07/06/1 > > Hello Kees, > > If I change BUG_ON() to VM_BUG_ON(), this check will work at least on Fedora > since it has CONFIG_DEBUG_VM enabled. Debian based distros have this option > disabled. Do you like that more than having this check under > CONFIG_FREELIST_HARDENED? I think there are two issues: first, this should likely be under CONFIG_FREELIST_HARDENED since Christoph hasn't wanted to make these changes enabled by default (if I'm understanding his earlier review comments to me). The second issue is what to DO when a double-free is discovered. Is there any way to make it safe/survivable? If not, I think it should just be BUG_ON(). If it can be made safe, then likely a WARN_ONCE and do whatever is needed to prevent the double-free. > If you insist on putting this check under CONFIG_FREELIST_HARDENED, should I > rebase onto your patch and send again? That would be preferred for me -- I'd like to have both checks. :) -Kees -- Kees Cook Pixel 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-18 20:04 ` Kees Cook @ 2017-07-19 8:38 ` Alexander Popov 2017-07-19 14:02 ` Christopher Lameter 1 sibling, 0 replies; 12+ messages in thread From: Alexander Popov @ 2017-07-19 8:38 UTC (permalink / raw) To: Kees Cook Cc: Christopher Lameter, Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, Linux-MM, LKML, kernel-hardening On 18.07.2017 23:04, Kees Cook wrote: > On Tue, Jul 18, 2017 at 12:56 PM, Alexander Popov <alex.popov@linux.com> wrote: >> On 17.07.2017 22:11, Kees Cook wrote: >>> Let's merge this with the proposed CONFIG_FREELIST_HARDENED, then the >>> performance change is behind a config, and we gain the rest of the >>> freelist protections at the same time: >>> >>> http://www.openwall.com/lists/kernel-hardening/2017/07/06/1 >> >> Hello Kees, >> >> If I change BUG_ON() to VM_BUG_ON(), this check will work at least on Fedora >> since it has CONFIG_DEBUG_VM enabled. Debian based distros have this option >> disabled. Do you like that more than having this check under >> CONFIG_FREELIST_HARDENED? > > I think there are two issues: first, this should likely be under > CONFIG_FREELIST_HARDENED since Christoph hasn't wanted to make these > changes enabled by default (if I'm understanding his earlier review > comments to me). Ok, I'll rebase onto FREELIST_HARDENED and test it all together. > The second issue is what to DO when a double-free is > discovered. Is there any way to make it safe/survivable? If not, I > think it should just be BUG_ON(). If it can be made safe, then likely > a WARN_ONCE and do whatever is needed to prevent the double-free. Please correct me if I'm wrong. It seems to me that double-free is a dangerous situation that indicates some serious kernel bug (which might be maliciously exploited). So I would not trust / rely on the process which experiences a double-free error in the kernel mode. But I guess the reaction to it should depend on the Linux kernel policy of handling faults. Is it defined explicitly? Anyway, if we try to mitigate the effect from a double-free error _here_ (for example, skip putting the duplicated object to the freelist), I think we should do the same for other cases of double-free and memory corruptions. >> If you insist on putting this check under CONFIG_FREELIST_HARDENED, should I >> rebase onto your patch and send again? > > That would be preferred for me -- I'd like to have both checks. :) Ok. Best regards, Alexander -- 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] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-18 20:04 ` Kees Cook 2017-07-19 8:38 ` Alexander Popov @ 2017-07-19 14:02 ` Christopher Lameter 1 sibling, 0 replies; 12+ messages in thread From: Christopher Lameter @ 2017-07-19 14:02 UTC (permalink / raw) To: Kees Cook Cc: Alexander Popov, Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, Linux-MM, LKML, kernel-hardening On Tue, 18 Jul 2017, Kees Cook wrote: > I think there are two issues: first, this should likely be under > CONFIG_FREELIST_HARDENED since Christoph hasn't wanted to make these > changes enabled by default (if I'm understanding his earlier review > comments to me). The second issue is what to DO when a double-free is > discovered. Is there any way to make it safe/survivable? If not, I The simple thing is to not free the object when you discover this. That is what the existing debugging code does. But you do not have an easy way to fail at the point in the code that is patched here. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 19:01 ` Alexander Popov 2017-07-17 19:11 ` Kees Cook @ 2017-07-18 14:57 ` Christopher Lameter 1 sibling, 0 replies; 12+ messages in thread From: Christopher Lameter @ 2017-07-18 14:57 UTC (permalink / raw) To: Alexander Popov Cc: Matthew Wilcox, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook On Mon, 17 Jul 2017, Alexander Popov wrote: > Christopher, if I change BUG_ON() to VM_BUG_ON(), it will be disabled by default > again, right? It will be enabled if the distro ships with VM debugging on by default. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption 2017-07-17 17:54 ` Matthew Wilcox 2017-07-17 18:04 ` Christopher Lameter @ 2017-07-17 18:23 ` Alexander Popov 1 sibling, 0 replies; 12+ messages in thread From: Alexander Popov @ 2017-07-17 18:23 UTC (permalink / raw) To: Matthew Wilcox Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim, Andrew Morton, linux-mm, linux-kernel, kernel-hardening, keescook On 17.07.2017 20:54, Matthew Wilcox wrote: > On Mon, Jul 17, 2017 at 07:45:07PM +0300, Alexander Popov wrote: >> Add an assertion similar to "fasttop" check in GNU C Library allocator: >> an object added to a singly linked freelist should not point to itself. >> That helps to detect some double free errors (e.g. CVE-2017-2636) without >> slub_debug and KASAN. Testing with hackbench doesn't show any noticeable >> performance penalty. > >> { >> + BUG_ON(object == fp); /* naive detection of double free or corruption */ >> *(void **)(object + s->offset) = fp; >> } > > Is BUG() the best response to this situation? If it's a corruption, then > yes, but if we spot a double-free, then surely we should WARN() and return > without doing anything? Hello Matthew, Double-free leads to the memory corruption too, since the next two kmalloc() calls return the same address to their callers. And we can spot it early here. -- Alexander -- 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] 12+ messages in thread
end of thread, other threads:[~2017-07-19 14:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-17 16:45 [PATCH 1/1] mm/slub.c: add a naive detection of double free or corruption Alexander Popov 2017-07-17 16:57 ` Christopher Lameter 2017-07-17 17:54 ` Matthew Wilcox 2017-07-17 18:04 ` Christopher Lameter 2017-07-17 19:01 ` Alexander Popov 2017-07-17 19:11 ` Kees Cook 2017-07-18 19:56 ` Alexander Popov 2017-07-18 20:04 ` Kees Cook 2017-07-19 8:38 ` Alexander Popov 2017-07-19 14:02 ` Christopher Lameter 2017-07-18 14:57 ` Christopher Lameter 2017-07-17 18:23 ` Alexander Popov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).