From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932766AbcGHRP0 (ORCPT ); Fri, 8 Jul 2016 13:15:26 -0400 Received: from mail-ve1eur01on0094.outbound.protection.outlook.com ([104.47.1.94]:37443 "EHLO EUR01-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932706AbcGHRPR (ORCPT ); Fri, 8 Jul 2016 13:15:17 -0400 X-Greylist: delayed 87685 seconds by postgrey-1.27 at vger.kernel.org; Fri, 08 Jul 2016 13:15:16 EDT Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=aryabinin@virtuozzo.com; Subject: Re: [PATCH v6] mm, kasan: switch SLUB to stackdepot, enable memory quarantine for SLUB To: Alexander Potapenko , , , , , , , , , References: <1467974210-117852-1-git-send-email-glider@google.com> CC: , , From: Andrey Ryabinin Message-ID: <577FDC2C.9030400@virtuozzo.com> Date: Fri, 8 Jul 2016 20:00:28 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1467974210-117852-1-git-send-email-glider@google.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.10] X-ClientProxiedBy: AM3PR07CA0036.eurprd07.prod.outlook.com (10.141.45.164) To DB6PR0801MB1301.eurprd08.prod.outlook.com (10.168.11.19) X-MS-Office365-Filtering-Correlation-Id: 2de4184d-84ae-4a04-181d-08d3a751382f X-Microsoft-Exchange-Diagnostics: 1;DB6PR0801MB1301;2:8JutyVGRw59J+cdAUx4EV6mxpwUcOh9g5EXhL9gWTXvRThNgBV+n1tGEYdCiEqNyZh4vK5OkWVExNhZK7g6572e1QoDSGtrwi8cj+3pCLCaxuW3/pBcXrkdUGv9tfWeEeZqutMlFsodApvChkMSyvZCTsIVt+Yojsy+ePwz1T+9ufE4mBvarsqDmsDvjPy2O;3:dYZHE9WnXCSTEznSq/YJfzUUJrQEC/lUajlIcWzc5W6eaAsltn8T9OUGwaNHtsu6G/c5cGVqxZsljN3mBp4CIc1sGLYDkQ5a03uuJ63qZk8HhAIfBX/k8qq/7fKlkCcv;25:tKRslj6d09S/qIgaf31iFiHfFQZ1YYUHxenBMWO332rhAAchoxQoFaB6kFWsjzeUVgERyLXaITq3Y3+xEXzc+3b0RGvFo/sQt7uSFhGklmWtowS9htQAnRFWTY20CDcNfnWYlq2rM3mFJJdYaZOiGCxg8RMdtDk2D/8+lWedp3oQUdBDh6BmW4b9XMRRyIZnrs5B5jBZoS2Mjc6zIbubLROsBbmu42kB69ZXwoNwnFUwoSfKc0dkDQ8rn5LxuoxSkKebzOQVK2a1Wxw0spB5jXX/X7kwMPqTQ2+LZpwaIVwJJZgRRjk9QhrKOcw68Tz1R5iaySQInKffX00Tt3+wY9/rMdSHAfrPA4r+RX62kOUHXSLdxoReJhyOT75SCV7UIDznLtyHXOOsWpYgWwfmV8skVcEtfYjjIgh9lJggooM= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0801MB1301; X-Microsoft-Exchange-Diagnostics: 1;DB6PR0801MB1301;31:7NgvJtunydWNnnqoi/Jhz+XlPinMCBFBzsqxRGBg2iJh96gtGBhtQDEAkQacMZK0J4jnQeOt9JruvRxWGp2/6aHQjlGHtOMFMc4Vc7PSsvk2s0H5tMf5vK08Qe5gkNu2fsJ0duzHBWkeie8Dfu09Tb39j2HwRD8dpBJQ93STAmaibSRt9YY6JPA4UKyMld74nM7Xo5bIOFjmntFKJhRC2g==;4:ZqdywqG7fv3b4yjiTrXzr/B145n/1SEOiVW0xPd8ibWn8MkACiPyigAeUjcOL99JKTe/fFB3ADibaelgGjq3p99r0ke7qkIuOrnr2MjK7v/MI4UhItEP4LD9TFJAdrKsvMC88Wnkum7MHTooXbR1a41lIBgki/sirhjmQZnniE8S/BzEZ6EoOkMabIW/XmBcznvviHZLjfEomcAuEjQsSnnjmZGPo8B74wi86yZX+ivcUTR1m9zqLsZtdPFB0ypElQgRfLeOi5XSYx6anw7Lg6QDvV8hTAL1Psny9OlDWrQ+r/GUrG5Aeb9Alll+MeFbyRS5Pr5/3GU7AFcTMMxAKbpVuMwREzjX1b8ab6l9lN6nFj57J8iXiRUg/vjQDDhaVq5txdo5J4ZdSnR9O8QhKyln5iDO/6VrvFMoc7MyBeA= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041072)(6043046);SRVR:DB6PR0801MB1301;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0801MB1301; X-Forefront-PRVS: 0997523C40 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(7916002)(189002)(199003)(24454002)(377454003)(106356001)(305945005)(65956001)(3846002)(105586002)(65806001)(586003)(47776003)(7736002)(66066001)(42186005)(83506001)(36756003)(2906002)(68736007)(50466002)(4326007)(6116002)(7846002)(23746002)(8676002)(81166006)(101416001)(92566002)(50986999)(76176999)(2950100001)(81156014)(97736004)(87266999)(189998001)(54356999)(86362001)(5001770100001)(4001350100001)(65816999)(230700001)(80316001)(2201001)(77096005)(64126003)(33656002)(59896002)(921003)(1121003)(83996005)(2101003);DIR:OUT;SFP:1102;SCL:1;SRVR:DB6PR0801MB1301;H:[10.30.19.223];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;DB6PR0801MB1301;23:Ay2bkJSiV4lLJyaA7WtpvGGgbkAbwlM7sjY?= =?Windows-1252?Q?gVxEXvZNvPwK2cWwKVuroGChF9wsEk+4MW/swuiSjBn5Icbo6uGmbxV9?= =?Windows-1252?Q?Uxiv+J/f4EhB/U46dHCEmZuwL+KzNwIk535HJw6kMOwSF4+dqNwzlwzR?= =?Windows-1252?Q?xXW7ZAogI8P+ibRZj1rXAjvPp+QXILJzouFkWetaFhe0PQSKeYmavFUz?= =?Windows-1252?Q?HEs9B3M9hJFqZ91IZeAq6ykIYxMZW8i3IjE1S6U5M28PAI0n3kYQRkTW?= =?Windows-1252?Q?KwAbplErtqkf29bN8xHspFS+tiLQJWUkdjLVgXSqaKyXO1VS4439pNA/?= =?Windows-1252?Q?vWkWbKhg272vQGpJi7o9qLHYuMoWqXED1q454eE1oRNx3qDljuBwG8fO?= =?Windows-1252?Q?Olm3Oxj0kTJ8Grp3rXFzls1BPAoLe5GhqXh1YPLKcAEwY58Oj69NUF7C?= =?Windows-1252?Q?ZNI39Fg9gNZ0Kqf26RBKEW/3fg4CiWXf9Duj2D2LAN6i74wJ7pC+fyFY?= =?Windows-1252?Q?m0SY8VJndKzmHNeIdeoVjw5Fnhl9oz4jBhHYMf3L/hkX1PIh8x8LZ60W?= =?Windows-1252?Q?FpLP8LjimZEEeUB0ua9h+mVD/t8MN5o2sVssirn8lmZmymMT/DkBlOxj?= =?Windows-1252?Q?NijEBdyIhOFh+MBo9U42Tl32a5rPKRjk/KLSkGJsGh9dO7jJI8e6313L?= =?Windows-1252?Q?yWhhGB95dUK+TIKvA129+C5vyhKyr3YEBtsiuVTANg+1+sY3/lctMLQo?= =?Windows-1252?Q?5yk5aiXmlcNwWc0kghlkOrE19m62IYzci5+kYWZ5AitD2UbxzTXVgei0?= =?Windows-1252?Q?WbhOcIULdvHV5o+aQNoPKxl/m2r1OVSzezu+smNSt/6lMNioLR1CYAIM?= =?Windows-1252?Q?siH9okIH4hOqX+jU/58+22TM5NZTFsMjoZPWkgo3iflSdYBCXdju9GBz?= =?Windows-1252?Q?PHY/UCenccziX2sllyJ8YVI/8Nzney14xckWX24cicEHKtDCVuIYOQTR?= =?Windows-1252?Q?toVUU5eOi9AopTpltzWnNoqLwm3/rNh6qqDK2ZxKYn+q/bdACGynX+9e?= =?Windows-1252?Q?xZkSv0uo2D0/igNbrcKU0zpbrBgv/rHwt0EDRMgJfSnOykC+Z82JbSvI?= =?Windows-1252?Q?9lSLokSoaKJLKoU0EtGZwJIELBf0HtEUKOsftyMJ6LJpxxd4vwcNAhib?= =?Windows-1252?Q?lN+JXjKERY2sFR4BdJM2i/ATNJVASuCSUtSl7HYJExHAT9O70zCy/1eH?= =?Windows-1252?Q?d9IoJI+SpnbhTfT69Fcav0lwgPsEzXdclZrQj+t7Fn/TtVYEpZP4fIq/?= =?Windows-1252?Q?mJrZMcP5B2lSuu125LCQf3R43uN5yEsRYpBEURp+pyrBjlzU4Nhh+WdO?= =?Windows-1252?Q?5LjpXWjxV8rihS+T9+3xJ1Duhr3ZFcJMwh7pm9EkvMEXkHh7A9SbU5sf?= =?Windows-1252?Q?aOKl7fREbguv5OKNygFZh?= X-Microsoft-Exchange-Diagnostics: 1;DB6PR0801MB1301;6:UblXCHKlWyv39KFZlUdHD6wMzQshqhqGNhZu/ckIKDj7LkvRgeSW5VKmadhh/QanJcLDAA8iXepRrh1rb/yjs9+vSTPZhkwzYwN7tuLkdiWa0gWFKe81pV0+6EcbiI4rq19vBKdjPXzn6ggfQwVXYSXobkIXByR+P/Xvye85BFPW2vJxHvcrhZY2vkjqBb9V04S9G30FdBGCL6nKq4te5UTlxT6pFKLDuSs48TYeUJuL/CnHjKh/F+/MAi2gyeyBiCReHSx/HsUOtTat+0/RiBuG/NXZl/LCHNlpYRL8+x/+mC243Uh8GygSjyA/sZFZ;5:4ao8VwHUKDnilaVpoOIHPrK26nChtdkoNZvfp/qnorZsuysoWCx9e3HZ8P3DnwAAyAEyeqnZyPzDaJkz6NdOflR9KztFW8+h672IkN3WDVaV41Fq+j5d/+42bjFGr13fWz2iIHvgIjjdAkMGaeSw+w==;24:pDavIabwf6y+SRHuZlC4mm/gIiX/ymb9IQ7eOeQzVRvx0x1XGGaTmyO18UAHSPYPVb0Qbwv+4yldLYROMsffWhd4V7SvacAX4JOQP/o4mkM=;7:wR8H8E0ox3/00ZV9Kk/a9RV/hNbxyU8Pt3klePe5nCQs9OFwv2jQcxqJ3PZEN196SblRHwRZiyjlyMIIKn2EjZ80+sqdwA0oWSyYiE6J7vmxAq+z2MU1pdtBAKfB+KMgZSn1QVH+KPYBOCjnzt5Xs7AO0DR9L0brUOjFR6dsDRA06sX4BAOPBpqD7XxkX9GhChtpV9VabMqt79zX+/kP8ArEKn+aD2nFlKtqDvzRtWtEVMhXoIZPPBIqF/J81PaZ SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DB6PR0801MB1301;20:urNmHRWS9iH5pj5P+nMlDKHtMza7r1rnESevoKiaNvcWK6vdO/YUOpOrEoUWbAx9l2CCq5RMzRXtLcsOj/ROzNIvGXkLOcjR5sOZLQTM1h+Y2oMJXvAPG6mTkn3AshyOCrBjl6VxBk6D7eq194GV/lyaeuuJynJ1e8Z8jYDXrZ4= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jul 2016 16:59:25.3221 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0801MB1301 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/08/2016 01:36 PM, Alexander Potapenko wrote: > > diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h > index d1faa01..07e4549 100644 > --- a/include/linux/slub_def.h > +++ b/include/linux/slub_def.h > @@ -99,6 +99,10 @@ struct kmem_cache { > */ > int remote_node_defrag_ratio; > #endif > +#ifdef CONFIG_KASAN > + struct kasan_cache kasan_info; > +#endif > + > struct kmem_cache_node *node[MAX_NUMNODES]; > }; > > @@ -119,10 +123,11 @@ static inline void *nearest_obj(struct kmem_cache *cache, struct page *page, > void *object = x - (x - page_address(page)) % cache->size; > void *last_object = page_address(page) + > (page->objects - 1) * cache->size; > - if (unlikely(object > last_object)) > - return last_object; > - else > - return object; > + void *result = (unlikely(object > last_object)) ? last_object : object; > + > + if (cache->flags & SLAB_RED_ZONE) > + return ((char *)result + cache->red_left_pad); > + return result; This fixes existing problem. It should be a separate patch. > > void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size, > @@ -568,6 +572,9 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size, > if (unlikely(object == NULL)) > return; > > + if (!(cache->flags & SLAB_KASAN)) > + return; > + I still think that this hunk should be removed. > redzone_start = round_up((unsigned long)(object + size), > KASAN_SHADOW_SCALE_SIZE); > redzone_end = round_up((unsigned long)object + cache->object_size, > @@ -576,7 +583,6 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size, > kasan_unpoison_shadow(object, size); > kasan_poison_shadow((void *)redzone_start, redzone_end - redzone_start, > KASAN_KMALLOC_REDZONE); > -#ifdef CONFIG_SLAB > if (cache->flags & SLAB_KASAN) { > struct kasan_alloc_meta *alloc_info = > get_alloc_info(cache, object); > @@ -585,7 +591,6 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size, > alloc_info->alloc_size = size; > set_track(&alloc_info->track, flags); > } > -#endif > } > EXPORT_SYMBOL(kasan_kmalloc); > ... > diff --git a/mm/slab.h b/mm/slab.h > index dedb1a9..9a09d06 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -366,6 +366,8 @@ static inline size_t slab_ksize(const struct kmem_cache *s) > if (s->flags & (SLAB_RED_ZONE | SLAB_POISON)) > return s->object_size; > # endif > + if (s->flags & SLAB_KASAN) > + return s->object_size; > /* > * If we have the need to store the freelist pointer > * back there or track user information then we can > @@ -462,6 +464,7 @@ void *slab_next(struct seq_file *m, void *p, loff_t *pos); > void slab_stop(struct seq_file *m, void *p); > int memcg_slab_show(struct seq_file *m, void *p); > > -void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr); > +void *nearest_obj(struct kmem_cache *cache, struct page *page, void *x); Remove. > +void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr); > #endif /* MM_SLAB_H */ > diff --git a/mm/slub.c b/mm/slub.c > index 825ff45..72ecffa 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -454,8 +454,6 @@ static inline void *restore_red_left(struct kmem_cache *s, void *p) > */ > #if defined(CONFIG_SLUB_DEBUG_ON) > static int slub_debug = DEBUG_DEFAULT_FLAGS; > -#elif defined(CONFIG_KASAN) > -static int slub_debug = SLAB_STORE_USER; > #else > static int slub_debug; > #endif > @@ -783,6 +781,14 @@ static int check_pad_bytes(struct kmem_cache *s, struct page *page, u8 *p) > /* Freepointer is placed after the object. */ > off += sizeof(void *); > > +#ifdef CONFIG_KASAN > + if (s->kasan_info.alloc_meta_offset) > + off += sizeof(struct kasan_alloc_meta); > + > + if (s->kasan_info.free_meta_offset) > + off += sizeof(struct kasan_free_meta); > +#endif That's ugly. Following is not ugly: off += kasan_metadata_size(); > if (s->flags & SLAB_STORE_USER) > /* We also have user information there */ > off += 2 * sizeof(struct track); > @@ -1322,7 +1328,7 @@ static inline void kfree_hook(const void *x) > kasan_kfree_large(x); > } > > -static inline void slab_free_hook(struct kmem_cache *s, void *x) > +static inline bool slab_free_hook(struct kmem_cache *s, void *x) > { > kmemleak_free_recursive(x, s->flags); > > @@ -1344,7 +1350,7 @@ static inline void slab_free_hook(struct kmem_cache *s, void *x) > if (!(s->flags & SLAB_DEBUG_OBJECTS)) > debug_check_no_obj_freed(x, s->object_size); > > - kasan_slab_free(s, x); > + return kasan_slab_free(s, x); > } Change it back, return value is not unused anymore. > > static inline void slab_free_freelist_hook(struct kmem_cache *s, > @@ -2753,6 +2759,9 @@ slab_empty: > discard_slab(s, page); > } > > +static void do_slab_free(struct kmem_cache *s, struct page *page, > + void *head, void *tail, int cnt, unsigned long addr); > + You can just place slab_free() after do_slab_free(). > /* > * Fastpath with forced inlining to produce a kfree and kmem_cache_free that > * can perform fastpath freeing without additional function calls. > @@ -2772,12 +2781,23 @@ static __always_inline void slab_free(struct kmem_cache *s, struct page *page, > void *head, void *tail, int cnt, > unsigned long addr) > { > + slab_free_freelist_hook(s, head, tail); > + /* > + * slab_free_freelist_hook() could have put the items into quarantine. > + * If so, no need to free them. > + */ > + if (s->flags & SLAB_KASAN && !(s->flags & SLAB_DESTROY_BY_RCU)) > + return; > + do_slab_free(s, page, head, tail, cnt, addr); > +} > + > +static __always_inline void do_slab_free(struct kmem_cache *s, > + struct page *page, void *head, void *tail, > + int cnt, unsigned long addr) > +{ > void *tail_obj = tail ? : head; > struct kmem_cache_cpu *c; > unsigned long tid; > - > - slab_free_freelist_hook(s, head, tail); > - > redo: > /* > * Determine the currently cpus per cpu slab. > @@ -2811,6 +2831,11 @@ redo: > > } > > +void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr) > +{ > + do_slab_free(cache, virt_to_head_page(x), x, NULL, 1, addr); > +} > + Probably would be better to hide this under #ifdef CONFIG_KASAN. It has no other users, and it might be relatively large function because do_slab_free() is always inlined. > void kmem_cache_free(struct kmem_cache *s, void *x) > { > s = cache_from_obj(s, x); > @@ -3252,7 +3277,7 @@ static void set_min_partial(struct kmem_cache *s, unsigned long min) > static int calculate_sizes(struct kmem_cache *s, int forced_order) > { > unsigned long flags = s->flags; > - unsigned long size = s->object_size; > + size_t size = s->object_size; > int order; > > /* > @@ -3311,7 +3336,10 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > * the object. > */ > size += 2 * sizeof(struct track); > +#endif > > + kasan_cache_create(s, &size, &s->flags); > +#ifdef CONFIG_SLUB_DEBUG > if (flags & SLAB_RED_ZONE) { > /* > * Add some empty padding so that we can catch >