From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A5D9C61D92 for ; Wed, 22 Nov 2023 02:28:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE2DB8D002B; Tue, 21 Nov 2023 21:28:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E93628D0008; Tue, 21 Nov 2023 21:28:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D5A938D002B; Tue, 21 Nov 2023 21:28:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C31F58D0008 for ; Tue, 21 Nov 2023 21:28:09 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 80BBF1605D5 for ; Wed, 22 Nov 2023 02:28:09 +0000 (UTC) X-FDA: 81484005498.27.052F7C2 Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by imf10.hostedemail.com (Postfix) with ESMTP id B45D9C0012 for ; Wed, 22 Nov 2023 02:28:07 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=URoXDqmz; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1700620087; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kfTZswEnYt7aHpTJOx6kD98hPK28QoQ/YumgrlaBkC0=; b=4yZIQKZEdiNB66013ELtINp4RWPtTvbfBUpoKkiD5i8tCNNmPKY4WOxTNBTTyROeB2naAK dTt+gW2Koqxd6Ip+bOpDEiI05/XF3P+ECorfFFAF8JTwWXL2SdP9lhOh3l1VsWDBeuVlre lf6rG8ruOYpeO4xKqFoUEke6f7shaww= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=URoXDqmz; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700620087; a=rsa-sha256; cv=none; b=uX21GNAxL5W40wxB39p7GeMXYj5x1aeTS8JPZ686z96+ErBzRrW3gg9mriJbOIEAOU+OAX d1N1sl4t9EHpnhJYOSKVp9ELalU8kvLPGqT8fv9wR7k6IygpzXvguV465D0KeiqLMKdVmj Kt47ODrjycpLrVidsq012li8uzkf2Y8= Received: by mail-pj1-f52.google.com with SMTP id 98e67ed59e1d1-283a0b0bd42so308521a91.0 for ; Tue, 21 Nov 2023 18:28:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700620086; x=1701224886; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=kfTZswEnYt7aHpTJOx6kD98hPK28QoQ/YumgrlaBkC0=; b=URoXDqmzKl4Xo7IiTWjcbfafL/Arxf2p05lbMv4WZetXuAYAaK3wpy6gsiChYxmhUz rLCM1iSlHr+3PfYTPulC/F1VeL9XHu+Iq5h/jzCs445kVTRUlXklL179nvWCD7O6A93q Cy9HMo2Gp3Ly+39LigzsGiPnBJA18BUGnqNHEMik5lqv3dydJhcSPuukRQA3kv8weJ29 TRgURC1OTmDWpRVHtLyH3fMbYE9fxXIxaS/sSKguSlKcTVDpAOsd/eqxZHOKSQpXLYOp AY5F5XpXHtTH4ggXNwWMN7O7dhUvkJ43xMLcUs6DN5y+e4A70Yo11paJVsoCG5YQhrIw jUTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700620086; x=1701224886; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kfTZswEnYt7aHpTJOx6kD98hPK28QoQ/YumgrlaBkC0=; b=ZxN/JSgR3ULWnw0AEJVQXkFTP5of9a8QbCz/ax+mO8Ee2XyCh4BNM+8Fvgqqd2H19N Vts6Qg3EWRUROLVS3+3gqA053EHSeZsjyqNgbvOYYkbSX4q5LvDnUiY6u4MVcbseiAMk OuadKV9sh7ZWDcrW+Ecl8wBnspdIxqT4TIR8M4UFfQExRdX3ACs+vI0HDYXwyVClEtYL 39okshTuGeIkEZghdOrvvaizrGRoamPLOTWXdovc1TIkdPB+VKiLe4qzk9drEmZc364o MfUe9h09R2W10mENeSwBuRW7dKfhz5UuqqtNRBHHP0/mEluKlcw3NzbZz3+eBplCsbP7 CDLA== X-Gm-Message-State: AOJu0YyBvfwBLHOocHxaMZayEIROK1xmdQ3osKJ8UJn5PMnIRlpC3BA0 S/BJoBT8rj9rW1uwHIigzBXxHtdpzWfBXwSDA+JerqkO8Lw= X-Google-Smtp-Source: AGHT+IHAMQ+tOeNRO2k3sRkUVDLcsPWJuo4/OHgwq3ka2J+5TxYjwdPReGcf9/rYE3eK82vEcdhwoQgJh8lBrOuobi0= X-Received: by 2002:a17:90a:fe90:b0:280:4a23:3c84 with SMTP id co16-20020a17090afe9000b002804a233c84mr6491607pjb.22.1700620086495; Tue, 21 Nov 2023 18:28:06 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Andrey Konovalov Date: Wed, 22 Nov 2023 03:27:55 +0100 Message-ID: Subject: Re: [PATCH v2] kasan: Improve free meta storage in Generic KASAN To: Juntong Deng Cc: ryabinin.a.a@gmail.com, glider@google.com, dvyukov@google.com, vincenzo.frascino@arm.com, akpm@linux-foundation.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kernel-mentees@lists.linuxfoundation.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B45D9C0012 X-Stat-Signature: 4m7f6ti3x43zo7j5ytpebgqbyow1skxq X-HE-Tag: 1700620087-94945 X-HE-Meta: U2FsdGVkX18E5pH5v9TkLB0XaOI1TVxxxoeBck8MvzV/nmC2vSFxG51P8JHapt+93Z/ATciviRaFIwzSUu9Vu8Y7hXWKg8zsWfCnI8GkW10aXE8lR8+fHMMUFongLhlV4CYN8+6OBvkSzkw7f9V339asfrhcoNh2Aodwmbh5oBDO/feGmP/Zf6pTcw31eAf4NdQw1cTT9g+qiJ96AkI0NmLA/iaPn7bpXlK3ger7Z5TFQy0BK+E4Y53NWfrKFgypEefaLsux7JidRNH9D87fy9BuV9PF6kMIKy7nJokmNKLNfFPwEcDCG3FaORoC3qQwWxs42BvQS7IpVuS9Cn8wMuWu6ovbqcxUf8MJ24+vtDjRq+Os019aLuyg2TMat3NeqDUGf4LsWoKObuwVJze97iwe/aLq64yTdpqvKry9CbMp6HFA6KNGyjxAxoOu44Np9DbcxAprrFEOJURv+H/I5LBC6EZVPCWAsCmUWVL3OQrKP+e2VuW8KhaAijiNn5v3cuIwonn5IUo9MAxljWd7RH5EB0v/kJwhqIXOPaJqAMKOzfkC2rTPFBSKza1tAb7CTJ8GpNtUhXuevoNQchNP7XJVbgcfzi1kCteoE5+aKhMWaLAF9I1kSJ2gI1ouZQ4G0P0CsicZGL74m4l3fc/KeTk/9IvnXmWTlEK17WpBXKGQfQimQtjSQOp0R188s5tyKjK8/XcYaFpFiJoeeHyrWF1XYkJ5RVbIbOg/okp/KK++PngLnAWcMhtlMgjws8Pbl0y7wUOiHkXsVEMAfvLHUlTuGOh4n3u/vlyyucLsfNk/oO3GTgyU/XamIrBFTfK/G46a6I0XhZezY/4hkWMakbS/9otTzGfP3TRcodDOACNd4kkO/aUKhSCTUD3xd3I1/QuHVdVwTPqImDW+lQISksTZSDSRJ0CW8I1ZV33wSazrBjWkWYLB5lecAYI6XZsH9EUl3CGTqRSNhrxf4fl VbHC2tdc dRuFOPr2lOoWfxbEftOF42z5YdTJKc4y+DCQzctMa7S+sK/P0IkhIwLpr3u2BBwdiqQDxZ68DIuT3zflqUH7qydLCABq9wLvnxwQpgNn61q9unJQ+eP5XqQAUZ5AR+55oyIdAmDjqDRGIOeHHCJshv8KHaMo7j9gjEpKfWen0X82IM/KZw0jmdfksnioQPj+/oouS7iOm/BD3Wq4RIXSS97JpcGCDg8pPz8DG15yax+ROZzYpslXz49hNiFNGtMi+HcO4dq1wT28lQhDaFrE4mb9om2JpnFn4w7k6scm0Asd2QDlnh8kgBzK4OVzB2LbFrQ1ofUn0dI4pVr80zOwGyE7cP/W+I/lTIQXHsWVA/v7X+pH57wND1zAulocenS9ao5Tob1Ubxx+X6Xq1lYWVgCy5D2UA274PldBX8x5yJfsdMcWe2LuqWcEACeq4pCE6ElRSeGL91oq3CufHVHJ4Eck6/fb5i623tvTMjLbGm8s0i1o8VdtPcMY+4w== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000177, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Nov 21, 2023 at 10:42=E2=80=AFPM Juntong Deng wrote: > > Currently free meta can only be stored in object if the object is > not smaller than free meta. > > After the improvement, even when the object is smaller than free meta, > it is still possible to store part of the free meta in the object, > reducing the increased size of the redzone. > > Example: > > free meta size: 16 bytes > alloc meta size: 16 bytes > object size: 8 bytes > optimal redzone size (object_size <=3D 64): 16 bytes > > Before improvement: > actual redzone size =3D alloc meta size + free meta size =3D 32 bytes > > After improvement: > actual redzone size =3D alloc meta size + (free meta size - object size) > =3D 24 bytes > > Suggested-by: Dmitry Vyukov > Signed-off-by: Juntong Deng I think this change as is does not work well with slub_debug. slub_debug puts its metadata (redzone, tracks, and orig_size) right after the object (see calculate_sizes and the comment before check_pad_bytes). With the current code, KASAN's free meta either fits within the object or is placed after the slub_debug metadata and everything works well. With this change, KASAN's free meta tail goes right past object_size, overlaps with the slub_debug metadata, and thus can corrupt it. Thus, to make this patch work properly, we need to carefully think about all metadatas layout and teach slub_debug that KASAN's free meta can go past object_size. Possibly, adjusting s->inuse by the size of KASAN's metas (along with moving kasan_cache_create and fixing up set_orig_size) would be enough. But I'm not familiar with the slub_debug code enough to be sure. If you decide to proceed with improving this change, I've left some comments for the current code below. Thank you! > --- > V1 -> V2: Make kasan_metadata_size() adapt to the improved > free meta storage > > mm/kasan/generic.c | 50 +++++++++++++++++++++++++++++++--------------- > 1 file changed, 34 insertions(+), 16 deletions(-) > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index 4d837ab83f08..802c738738d7 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -361,6 +361,8 @@ void kasan_cache_create(struct kmem_cache *cache, uns= igned int *size, > { > unsigned int ok_size; > unsigned int optimal_size; > + unsigned int rem_free_meta_size; > + unsigned int orig_alloc_meta_offset; > > if (!kasan_requires_meta()) > return; > @@ -394,6 +396,9 @@ void kasan_cache_create(struct kmem_cache *cache, uns= igned int *size, > /* Continue, since free meta might still fit. */ > } > > + ok_size =3D *size; > + orig_alloc_meta_offset =3D cache->kasan_info.alloc_meta_offset; > + > /* > * Add free meta into redzone when it's not possible to store > * it in the object. This is the case when: > @@ -401,21 +406,26 @@ void kasan_cache_create(struct kmem_cache *cache, u= nsigned int *size, > * be touched after it was freed, or > * 2. Object has a constructor, which means it's expected to > * retain its content until the next allocation, or Please drop "or" on the line above. > - * 3. Object is too small. > * Otherwise cache->kasan_info.free_meta_offset =3D 0 is implied. > + * Even if the object is smaller than free meta, it is still > + * possible to store part of the free meta in the object. > */ > - if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor || > - cache->object_size < sizeof(struct kasan_free_meta)) { > - ok_size =3D *size; > - > + if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor) { > cache->kasan_info.free_meta_offset =3D *size; > *size +=3D sizeof(struct kasan_free_meta); > + } else if (cache->object_size < sizeof(struct kasan_free_meta)) { > + rem_free_meta_size =3D sizeof(struct kasan_free_meta) - > + cache->ob= ject_size; > + *size +=3D rem_free_meta_size; > + if (cache->kasan_info.alloc_meta_offset !=3D 0) > + cache->kasan_info.alloc_meta_offset +=3D rem_free= _meta_size; > + } > > - /* If free meta doesn't fit, don't add it. */ > - if (*size > KMALLOC_MAX_SIZE) { > - cache->kasan_info.free_meta_offset =3D KASAN_NO_F= REE_META; > - *size =3D ok_size; > - } > + /* If free meta doesn't fit, don't add it. */ > + if (*size > KMALLOC_MAX_SIZE) { > + cache->kasan_info.free_meta_offset =3D KASAN_NO_FREE_META= ; > + cache->kasan_info.alloc_meta_offset =3D orig_alloc_meta_o= ffset; > + *size =3D ok_size; > } > > /* Calculate size with optimal redzone. */ > @@ -464,12 +474,20 @@ size_t kasan_metadata_size(struct kmem_cache *cache= , bool in_object) > if (in_object) > return (info->free_meta_offset ? > 0 : sizeof(struct kasan_free_meta)); This needs to be changed as well to something like min(cache->object, sizeof(struct kasan_free_meta)). However, with the slub_debug conflicts I mentioned above, we might need to change this to something else. > - else > - return (info->alloc_meta_offset ? > - sizeof(struct kasan_alloc_meta) : 0) + > - ((info->free_meta_offset && > - info->free_meta_offset !=3D KASAN_NO_FREE_META) ? > - sizeof(struct kasan_free_meta) : 0); > + else { > + size_t alloc_meta_size =3D info->alloc_meta_offset ? > + sizeof(st= ruct kasan_alloc_meta) : 0; > + size_t free_meta_size =3D 0; > + > + if (info->free_meta_offset !=3D KASAN_NO_FREE_META) { > + if (info->free_meta_offset) > + free_meta_size =3D sizeof(struct kasan_fr= ee_meta); > + else if (cache->object_size < sizeof(struct kasan= _free_meta)) > + free_meta_size =3D sizeof(struct kasan_fr= ee_meta) - > + c= ache->object_size; > + } > + return alloc_meta_size + free_meta_size; > + } > } > > static void __kasan_record_aux_stack(void *addr, bool can_alloc) > -- > 2.39.2