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 48E87C4332F for ; Thu, 14 Dec 2023 08:35:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C317F6B0565; Thu, 14 Dec 2023 03:35:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BE21D6B0567; Thu, 14 Dec 2023 03:35:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A81D56B0568; Thu, 14 Dec 2023 03:35:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 901F46B0565 for ; Thu, 14 Dec 2023 03:35:34 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 6075FC018C for ; Thu, 14 Dec 2023 08:35:34 +0000 (UTC) X-FDA: 81564764988.26.84A98B2 Received: from mail-vs1-f54.google.com (mail-vs1-f54.google.com [209.85.217.54]) by imf23.hostedemail.com (Postfix) with ESMTP id A12A7140013 for ; Thu, 14 Dec 2023 08:35:32 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=HSNgs+jN; spf=pass (imf23.hostedemail.com: domain of elver@google.com designates 209.85.217.54 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702542932; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=59EvNi4lROlx/uAEwIQ9N8BChY4yKEZ29O9hKyzJ3t8=; b=mbKBGmt/ExBAHxo4K0Za3ZghG7t4yn+9ZL/9Md2asLZNN2kPIaX6KHxIvj0s2Zy4aUV/PH jjhSkXH9+xbA+PF7E/ToJm7hecZ7Qh4FqY08msfn7KTpGClKxWkaVKzqpL/hK7qZen6TA5 +N23zzuCmE++YOfoe2AQu3qHffW2V28= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702542932; a=rsa-sha256; cv=none; b=q+gy2cxZlDGr/QuJexPM7BeiDsFJiVYwp+2tQ7ldYZtqsMVLsVyZSQFV7aAMbtasTmsdoX CJch+bvGinJSyENVIu+AABmO4UtAZ+nDtfsghmzmP77AyzNp+RM3veo/s9MnwO4e8Tq7Ax ZV/CQSfKByrnnoMC/QbJtgQIyP+wsJg= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=HSNgs+jN; spf=pass (imf23.hostedemail.com: domain of elver@google.com designates 209.85.217.54 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-vs1-f54.google.com with SMTP id ada2fe7eead31-4649d22b78aso227851137.0 for ; Thu, 14 Dec 2023 00:35:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702542932; x=1703147732; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=59EvNi4lROlx/uAEwIQ9N8BChY4yKEZ29O9hKyzJ3t8=; b=HSNgs+jNib60HNifT8UblKkn+EgW4fvg08bG/9kdSJKoH3pzjAqmsHu4V+8UieLZK6 GQgmJ112bzFlU10Q2igJLQ4sQiBVQ235rBS2rj8V7Udrz9S5HR23XDVO6DtRNHrzMNq7 UFDU7MYzGTG7VyqIiQsIISzsdCgvInEVBuBtZHvcq+3lUb0yGaangBDpIdVPsmLL5Gbb E1TiB6plAmkFGbsEo1GSkCGg5yaYfbMdciZeRQQD9qAGYn1AAz2VZ6py/1lfUl/ISM2h v28iIIdBP1fZPANtV4FcCiNawGvj/jLCcEY/TL3+1GlGYkMWCIKW3tRhCDPTY1zpLWUr rnvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702542932; x=1703147732; h=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=59EvNi4lROlx/uAEwIQ9N8BChY4yKEZ29O9hKyzJ3t8=; b=eSZhchsS2XzhIgr+yhkvCqMqwcN5kTHoizohUjy2Ecf9WVlT1FAiYLaeRFGgrXGaqd 7jZ17hsCyEg1G8dQlDBPPKcxvXsN5oa4r5zd9GJW/UpB9HFVbHJUogbwC8mSKk6blhVq 412mlQEarEWuuW7zPLPmXyPJBXJVaIkYg5GZet6RPRRxAgl6N3XTSOlAh1UmpygLBWOY Q0uW9nZPaa3gQ+msy7I6v3Pk1GeFHu7BzbWEwambExo1k5ZOaI1aBgjaRrApA6P0F+ci RMWaFyDAa0RPWH3iLfTAjAYywmzPFlXdkWBUPEKnIeLjx9PYX/3WNRKIR0w6NZbbNXCg M4CA== X-Gm-Message-State: AOJu0Yx3fmpyUEQCQJL4d+shN8I4HRpKd8wM1+qddbffFic61c/y0hjt xe8r5OaoKOFIVleo2FEb/NRfamEGX7mPzxDDIq6tNA== X-Google-Smtp-Source: AGHT+IFjHIPGCGlzsnBtrnOvnWM1vx11pK89D8pyXHZ2WDEvqPofbyINSZK0oI29bXPM4O6KOZniRHiu5ZdLnpJuIK8= X-Received: by 2002:a05:6102:e0e:b0:466:25f:f281 with SMTP id o14-20020a0561020e0e00b00466025ff281mr8702541vst.6.1702542931628; Thu, 14 Dec 2023 00:35:31 -0800 (PST) MIME-Version: 1.0 References: <88fc85e2a8cca03f2bfcae76100d1a3d54eac840.1702514411.git.andreyknvl@google.com> In-Reply-To: <88fc85e2a8cca03f2bfcae76100d1a3d54eac840.1702514411.git.andreyknvl@google.com> From: Marco Elver Date: Thu, 14 Dec 2023 09:34:53 +0100 Message-ID: Subject: Re: [PATCH -v2 mm 2/4] kasan: handle concurrent kasan_record_aux_stack calls To: andrey.konovalov@linux.dev Cc: Andrew Morton , Andrey Konovalov , Alexander Potapenko , Dmitry Vyukov , Vlastimil Babka , kasan-dev@googlegroups.com, Evgenii Stepanov , Tetsuo Handa , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrey Konovalov , syzbot+186b55175d8360728234@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: A12A7140013 X-Rspam-User: X-Stat-Signature: 6px5xxik3x38epakb8q1nu6tmeicye11 X-Rspamd-Server: rspam03 X-HE-Tag: 1702542932-817619 X-HE-Meta: U2FsdGVkX184vCouqyBJ5t3rI+ynUhO2YEWIbBykmzH6wRit4P96qzXLJOsqOMsxmvi6eywJBm+EOrX46+FcwljIWcp3ZMEr2vHamxET0ytbVoBbwrhm1SZS1zByc8r915N6QH/RysaiWWBJiqhJbTGI0Hv+yDzy1VvrXuCfd6A7JoD3r0Oxu9Mksk5DmACj10nNqBQXhZxal1LPdfL6phWY+ps7lcWEvRovM+wGUdsRze2KgaTrAklzRjQ/ebEEFV3AkHcuUpgPhRAe16+rRALP1TBHHodLcpzV9chZ0lCKpxBwZzYXz7fjcFit1a3C50G69kqDiagrF5CFJ78WVKs+b+VdrwI4AIN+e64RuHRkjYFDax7vyoWDajEDO+gKyCvadJ/zkxij6EbylXQJKSNUFqDarSTRPf8y/HPm3Jckhhp8lB/GlBU/qiJ05n85Yqf5daJdCzFjpOl6WjgT2wlN2DMPJ4kYDjw7VHbFU/G7vXWguTSQ/Szx6hE+aTGWYjgQuU/7r5Yq6qJCp2aL96Ft1Po4fWnQtI23CExNXEEQxl9pIUgOsvmg5RD2NrygqpUhErnywzC2sfvbTmxF4qM4VgSmpv3pgeL64NS+fSg9oSndL6HJmXQu4J+fihFRqGO2YjsirH5/QKWspak5pWHWgbvytBc44yaEsLu63l78N+nv/ISkXWH5qYTTI0BvJI/JD2sr2HZRkHndqfYKpfF0k8Ae4Y69xmlxYk9uy/Q0KK7QHJYj2pApzQQxC5Tdu5pIXBkcxA/d/MOtJ1tjRZzgD77Rt9GrHXx+n7ujd7E/nTFeWnSSss/NWDMBS0icMRohnXr3VdkPrcD1Cob4RAbD2/wt5iS+MuQe7Ia+bgv9sO7bd2dxq6MfhCnkGxJtfCcnhl84yjX9wwM6sUmc3UnFMIvffN3M+48jGaJB1PVyc5uWDBAiG3VbnKQj/LxznPCeZ2/HlEK4UsmEX6P A4s60j48 pSkbnVi0H+Ffiob5vX4gzBhvwWkRCPOLhx2LUbaMHqsbhvF82rC90dydQSkbiZeBEQgJ85KnX8So7PLlMm9Aw4StcRueVNKQvdVHlsTsXGjTRu/iaVQwQvYNMkKHlkgaNp9JQXd/9TBA9W6HQWCMcJLUQBNvj5LR73ve9YwJSyUa5dRC/FJRHmOxwQ0gVi36/a+YBbMIFeUuICnffmgd4BpPzSmPdwjTQlc/jIHV2kZFX5sPMIlbrOE/AITDovnxE3/nQSHpK3fnx0jzGWfO/Bxr8SHyCxj8QvruyLhF/GFZcsTi3iPWsBvybc0S6AFKVrm22QiqezCYsl8Tbi7CmULAZe2MgkZ8IIWdtJMBvDA32PNvIEAwJP0kN++j+2unvFTSclGrsjQamH+txX9fnJR5JTqCffrSJ2ms+BtaHf1lWo7JjTp2khQP8Wn0NI9kuMNF1uzHJdSB21Dwi0HGVqlt5KLb0O8p21LrPcx+zSjMLQ2SwZlHVZBMF2V7fw8EbzCs2 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 Thu, 14 Dec 2023 at 01:48, wrote: > > From: Andrey Konovalov > > kasan_record_aux_stack can be called concurrently on the same object. > This might lead to a race condition when rotating the saved aux stack > trace handles, which in turns leads to incorrect accounting of stack > depot handles and refcount underflows in the stack depot code. > > Fix by introducing a spinlock to protect the aux stack trace handles > in kasan_record_aux_stack. > > Reported-by: Tetsuo Handa > Reported-by: syzbot+186b55175d8360728234@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/000000000000784b1c060b0074a2@google.com/ > Fixes: 773688a6cb24 ("kasan: use stack_depot_put for Generic mode") > Signed-off-by: Andrey Konovalov > > --- > > Changes v1->v2: > - Use per-object spinlock instead of a global one. > --- > mm/kasan/generic.c | 32 +++++++++++++++++++++++++++++--- > mm/kasan/kasan.h | 2 ++ > 2 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index 54e20b2bc3e1..b9d41d6c70fd 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -471,8 +472,18 @@ void kasan_init_object_meta(struct kmem_cache *cache, const void *object) > struct kasan_free_meta *free_meta; > > alloc_meta = kasan_get_alloc_meta(cache, object); > - if (alloc_meta) > + if (alloc_meta) { > __memset(alloc_meta, 0, sizeof(*alloc_meta)); > + > + /* > + * Temporarily disable KASAN bug reporting to allow instrumented > + * spin_lock_init to access aux_lock, which resides inside of a > + * redzone. > + */ > + kasan_disable_current(); > + spin_lock_init(&alloc_meta->aux_lock); > + kasan_enable_current(); > + } > free_meta = kasan_get_free_meta(cache, object); > if (free_meta) > __memset(free_meta, 0, sizeof(*free_meta)); > @@ -502,6 +513,8 @@ static void __kasan_record_aux_stack(void *addr, depot_flags_t depot_flags) > struct kmem_cache *cache; > struct kasan_alloc_meta *alloc_meta; > void *object; > + depot_stack_handle_t new_handle, old_handle; > + unsigned long flags; > > if (is_kfence_address(addr) || !slab) > return; > @@ -512,9 +525,22 @@ static void __kasan_record_aux_stack(void *addr, depot_flags_t depot_flags) > if (!alloc_meta) > return; > > - stack_depot_put(alloc_meta->aux_stack[1]); > + new_handle = kasan_save_stack(0, depot_flags); > + > + /* > + * Temporarily disable KASAN bug reporting to allow instrumented > + * spinlock functions to access aux_lock, which resides inside of a > + * redzone. > + */ > + kasan_disable_current(); > + spin_lock_irqsave(&alloc_meta->aux_lock, flags); > + old_handle = alloc_meta->aux_stack[1]; > alloc_meta->aux_stack[1] = alloc_meta->aux_stack[0]; > - alloc_meta->aux_stack[0] = kasan_save_stack(0, depot_flags); > + alloc_meta->aux_stack[0] = new_handle; > + spin_unlock_irqrestore(&alloc_meta->aux_lock, flags); > + kasan_enable_current(); > + > + stack_depot_put(old_handle); > } > > void kasan_record_aux_stack(void *addr) > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index 5e298e3ac909..8b4125fecdc7 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -6,6 +6,7 @@ > #include > #include > #include > +#include > #include > > #if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS) > @@ -249,6 +250,7 @@ struct kasan_global { > struct kasan_alloc_meta { > struct kasan_track alloc_track; > /* Free track is stored in kasan_free_meta. */ > + spinlock_t aux_lock; This needs to be raw_spinlock, because kasan_record_aux_stack_noalloc() can be called from non-sleepable contexts (otherwise lockdep will complain for RT kernels).