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 B1828C4332F for ; Wed, 13 Dec 2023 14:40:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1673A8D0022; Wed, 13 Dec 2023 09:40:49 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 116078D0015; Wed, 13 Dec 2023 09:40:49 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ED1988D0022; Wed, 13 Dec 2023 09:40:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id D82518D0015 for ; Wed, 13 Dec 2023 09:40:48 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 9CAD31A01D9 for ; Wed, 13 Dec 2023 14:40:48 +0000 (UTC) X-FDA: 81562056576.07.7877891 Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by imf05.hostedemail.com (Postfix) with ESMTP id D2101100014 for ; Wed, 13 Dec 2023 14:40:46 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PtuHIEn4; spf=pass (imf05.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702478446; 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=t3mJDSsBd+gLTGFYfrkyu9tBgFHp0rV5RVCzftSRiIQ=; b=jCQwK5A4h3XQ2pGbbqbTCWgdi/9gjk10lhdKSngi+c9PFj8eosfkcZ67bkA6SPuTZ+vzHd JS0ZEZBWfL+Z8FrpnKqeOjoflQHJcq8gkWHa/Bcuvg6qfrcHpA2VWrnphKO5bnJH51xB1l n0KsH4SdTgUo6dNLqgggGYaa1UyvIjs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702478446; a=rsa-sha256; cv=none; b=Xx1Cogys7A0Q+EdnJGOjLrKSh0ytsCd1ssMM3SNWbyA5yN52Xqi94vING/FbFSo2Gwsoxs MdX5ctgnWvnXybshOjwNBzH0f4OPkfnAXsBSXJo5a0cCGe0GQRB7vy2McaPMmCsYFXRf47 2dwTQordkyiAobSet4q1lIJRYQCzZjs= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PtuHIEn4; spf=pass (imf05.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pg1-f181.google.com with SMTP id 41be03b00d2f7-5c66b093b86so6099216a12.0 for ; Wed, 13 Dec 2023 06:40:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702478445; x=1703083245; 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=t3mJDSsBd+gLTGFYfrkyu9tBgFHp0rV5RVCzftSRiIQ=; b=PtuHIEn4L8yRyDflaRUk2+VvvcEWJBDF1zd288h5W6ve5+QVPIZlSD5O2Dy5oQJDU6 xuFz9Z3fBuz9pYToiLE1HYi2ef2Iq/mlzw71SJYE/Z/BZFW//sgv9yBcHXaDaBfhq34F HHqq9n1lrA75I+km5z2nEbfA9kU4kXRZkl5vmWGtZXuhx00y93IqjtbH0AREOUOPBmix 7evW9FjVR3PA2JXtVi4Kx5IDYnWv9CJ1A0Qc/oqLmivB0Sp/wnrlxcWWEKf8lUPqMNqZ NdIOBzOKIlHryq78d104zfB0g0sWBYw0Y+PwuWDGT/UZfDW6NPtSTmJuzTe1nA1KxmZ5 e6BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702478445; x=1703083245; 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=t3mJDSsBd+gLTGFYfrkyu9tBgFHp0rV5RVCzftSRiIQ=; b=u61opw8vo0+VrOLxZcEgeebjVTv6vzfv5midx9jnLqQlKzM9MtzkJDgzZ/OzkTU07n t/+fPLvaysDSu41zuQREVjddWFMdJspW0cK/CdmgquXo3wWacxcJjcFsyq0/lCK5Jvdg KT7aPImTcrZQtm5/ba16Pm1ZkKkAuttedIn6JM9cHM/NkYz7LIGifVfl01E36o9MiWSV H/4rplV86VF0ub2mfCcXOQ3HXq/k/8S9pi5WkXAvn+mhu3ZBrM25Wp+wnttjzWCGMdmc SDm/PvUuiq2VvPprcYCo3XZnhMxbdhEbO/Y+zi2X0t1OOxUKKG++77HgvTXZzq/QpHxB HE3A== X-Gm-Message-State: AOJu0Ywq8ArFT0JAgJHrdEeshz9MwZR5iTxZqNznP6y6uMDxHFICfqod 05+pC3WaNUhRYDtBA/914LOSRcmSbU1t15+ClLs= X-Google-Smtp-Source: AGHT+IF6caqmkOkKchSE4mSM69yXAgz5JoYHbVboHyzfw8Axgg+y5yFAUSGQz+qEjv5WV2j5pYSexT6qvPQj785jqew= X-Received: by 2002:a17:90b:e0e:b0:28a:dcda:a101 with SMTP id ge14-20020a17090b0e0e00b0028adcdaa101mr1926855pjb.47.1702478445662; Wed, 13 Dec 2023 06:40:45 -0800 (PST) MIME-Version: 1.0 References: <432a89fafce11244287c8af757e73a2eb22a5354.1702339432.git.andreyknvl@google.com> In-Reply-To: From: Andrey Konovalov Date: Wed, 13 Dec 2023 15:40:34 +0100 Message-ID: Subject: Re: [PATCH mm 2/4] kasan: handle concurrent kasan_record_aux_stack calls To: Marco Elver Cc: andrey.konovalov@linux.dev, Andrew Morton , 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" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D2101100014 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: q775jj13rtz3jqk9gzaygqt353kdsu8d X-HE-Tag: 1702478446-250366 X-HE-Meta: U2FsdGVkX18OqHOOm93Buh/IdD6aRaOVVW4YODNvuX9Sjxoufzmcrvp1t8ZBLEDLS2MHVcmtvRClTFTfhr/qaE6n4Yp+Pt8g+P4vPgMnJWzCvNyrC5VIvPQVj9EAPZGLPK4Nxkelky8zVDzsXeeDSau/kwjQVa8dPllKD4FYSgQlbNcPz3wnjFEtJTS14THD3CkBD+zplkC9VwO5U9EvzDBKsq1IJA7AyhxT/HSHDYqgfIwBRqWho/ZvQZZzFJ96G4ztxmbgDXaV+WH9Uyv9fBo4vwr+ccjtKmuLW0aF571o1JrNlUiZYAZjlukjQkEiyi8zL5Y/qKpnjqAVs3fdiZTH+YdDEUB4fBKXjhmpRZp8Fr6HYHBhlOOxUlda7zW2t3S25PzBR6QgVBZ9ZBRJvXtglWr0U9E+M68EzLYax+Jc73ZtvRqf7yuEZUJrkJZ4CgMYmHLb5KvtxfYrweOGS3jcaAvnf19otGtKxfnLelee/hgF4dMVxCheznaNuz6wMkMSXAQ/4HGTY406qGfNxnKeuItkEEmB6+TcstaJ5dr1HCYR5XNnIAHoJMg/qv5GBhrCUR6k+gCglQdxurwv+Ajhd3fpijT0KRBAtueel5ENlqMCFPtbwhayoFG+AVaG51l7GOW8n/Ni4KroRSKBfUvPCN/BZyct12ax544AWQzaQBjMYAmr9ILSCBAfZudnN+RiqQ+EEvNp6Zr5m42xKSI6+eRl52XtzsgivtcQ6N2VNzEEd9aSCeMi8aNa6EhCc3p2kvhs3y/NqSdNoTSewkM87v4q+8wXRWH1ddEsYXWK1REwVsNPPupbFX74aZRIzIC6DahnIXhBrNfQ+pC8LAFgtOCkPOT4RBEcUoAWyGF7baLO7ruEJFbrWsWrobvYEWiWVPmvYaq+cuCZR41KPIm7S87GIc919ZfopDNr0reUVOstLPAMzrrDEOpPsnfzLKVxc5Gm4n3VRorzk8D T28IRCVz kRjjMZmD3tCXOakZTXggeBJCDwhc6HqqfyJg08kAcOGcV8Jn0CcU32uEUsnOyssALyDj5v+B8MtktWhdviS/uAqSOaDOeghW6/RdKFem/jc6dW/cUy0fzQb7FS6w+jJS3aaTRCtFPFc9dw9OpcpJyR12jyHdkGyVl4eSaVRjKHTpUeXBWA3Xw6vurXyMztgAr+MNhz9imk7D6kgT+jn/LsWMh06H1JZL/rzG8wps+tMmY5HWbews00dcOTgk92fEK5pCxaOttPDAk+5kvEPEz5o1q0KTdDSQtknHMHDFWd52zX0g2rRBPM4Wme/373Wdl7DjCy8uzSDjN8/x9NfTMsH6AMZgH6avVwuZIpE+7tRnjcauJp2Xix4pOVIp+Bs6GYydFkEmLPhDE2T6gu/5dfdjhlurwW1ayPSfOtvqtMLFzNqryRJQTcMvZRGDeU3zGX7vKgTXAEV4NKhMekq4WrRyt9kB1PP2dlPQcwZAPP2wq6PYHpVZKNk9Zg3yajo6QFR+1UeKw+H8/Kc0= 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 Tue, Dec 12, 2023 at 8:29=E2=80=AFPM Marco Elver wrot= e: > > > - stack_depot_put(alloc_meta->aux_stack[1]); > > + new_handle =3D kasan_save_stack(0, depot_flags); > > + > > + spin_lock_irqsave(&aux_lock, flags); > > This is a unnecessary global lock. What's the problem here? As far as > I can understand a race is possible where we may end up with > duplicated or lost stack handles. Yes, this is the problem. And this leads to refcount underflows in the stack depot code, as we fail to keep precise track of the stack traces. > Since storing this information is best effort anyway, and bugs are > rare, a global lock protecting this is overkill. > > I'd just accept the racyness and use READ_ONCE() / WRITE_ONCE() just > to make sure we don't tear any reads/writes and the depot handles are > valid. This will help with the potential tears but will not help with the refcount issues. > There are other more complex schemes [1], but I think they are > overkill as well. > > [1]: Since a depot stack handle is just an u32, we can have a > > union { > depot_stack_handle_t handles[2]; > atomic64_t atomic_handle; > } aux_stack; > (BUILD_BUG_ON somewhere if sizeof handles and atomic_handle mismatch.) > > Then in the code here create the same union and load atomic_handle. > Swap handle[1] into handle[0] and write the new one in handles[1]. > Then do a cmpxchg loop to store the new atomic_handle. This approach should work. If you prefer, I can do this instead of a spinlo= ck. But we do need some kind of atomicity while rotating the aux handles to make sure nothing gets lost. Thanks!