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 96FB5C4332F for ; Wed, 13 Dec 2023 16:51:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C297D6B0496; Wed, 13 Dec 2023 11:51:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BDA9A6B0498; Wed, 13 Dec 2023 11:51:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7AD76B0499; Wed, 13 Dec 2023 11:51:17 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 950546B0496 for ; Wed, 13 Dec 2023 11:51:17 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6491C12015A for ; Wed, 13 Dec 2023 16:51:17 +0000 (UTC) X-FDA: 81562385394.11.3B84770 Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) by imf26.hostedemail.com (Postfix) with ESMTP id 94B36140023 for ; Wed, 13 Dec 2023 16:51:15 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OVqsi26L; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf26.hostedemail.com: domain of elver@google.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702486275; 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=+aZ9Gls1BogU2MReoFJauXP2UgnNIXaBIdamecIfCjI=; b=jfXY8Nwi/mZ5OX8NcauUoKZXFcu9MZnACH9fEt2vdDAz60Rsp4bGszwEPkgfrQSP8Y0Rrr bY2NIVkkMrDxqsbspC6EkjDc2CtRz2yPblqYANRN+ULkIkxzXuewdhCPbv+0nOwF5Wvaa4 kiBsocQpVnXEn4FlWk43MLOCf1IQzAM= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OVqsi26L; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf26.hostedemail.com: domain of elver@google.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702486275; a=rsa-sha256; cv=none; b=hFQOgEbv5UBIgbcwoTVKt0vmMwtzx5sT8PUa+5Ibtt7EeeeS/Ba604hFkosqhvk6jwO639 yQEW0cCRxDhqN91BAzlHah1AcC6t1ohK3AEUwyDWqnuFm0vovPfHa8WGXhW2zjzr6LRV8f sRUSPBdyi+pa/5PIfgGhRC/26Eh2Niw= Received: by mail-ua1-f44.google.com with SMTP id a1e0cc1a2514c-7cae73b1641so1117622241.1 for ; Wed, 13 Dec 2023 08:51:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702486274; x=1703091074; 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=+aZ9Gls1BogU2MReoFJauXP2UgnNIXaBIdamecIfCjI=; b=OVqsi26LvKI8JI85y5d94rUn+vE2NH6HOGzqgP1zoVkW5f6ldde4f7cDUuc1XRn9nf UvRMcl28bGtgmpJWAO+RCYWDtwEJ5tITzxWXtEkS2OY+kmIeMu+t0BGLPwHFQf+CE75+ /IcGz1SDaFgjV7Eyf8t49g/sGZK4wYEElupBoR7rhlBBD9ZIQViogGvUS/x0dr0hPeLJ BLqcfyUW90aMnSqQeBIS/yelAvP/tDAuBd8sjVYWKkb/JO8z1iF4tkGwqxqAkeYS8Gu5 IzgjFT8DaM6bQIv6/8dgQYtIH4+OjSPfPmQTNWTrkbwUnuE47tHniHZuobpxTE75Efua GStw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702486274; x=1703091074; 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=+aZ9Gls1BogU2MReoFJauXP2UgnNIXaBIdamecIfCjI=; b=dcZx2za5yyV4QIGRoI4TjVLaWSQ1Tka/GtGI2ACJbM3uOgScbwbahyeZvwV885s1q0 CnsyFC0wv8AfUebsld+6NHpsGqD5wiph0lGkqdw4hEcrqFxeIUwgr2x5HulWFgbi/c4X vJn/gg4dHTZ1jzfthVt6PV6YeUKNUdQAIme0FDZzX1NM6zjBh438jgNJIXcxZIGsKVea PQpxc2m+AF3Vl48y3tKIxZg1K03bKHocE9J3CuoVqtl2JcfCKnGg23wv9yqf3b0c5EuD bsh0ANR0iYak/NVi3vlyVtV6YkficAXwPOSEMfxl/X772jOvmwtLnJZsT+LAXfnPH5B1 R/QA== X-Gm-Message-State: AOJu0YxR0JoRxyPM67FidpDOODBXiQNv4AbRedrkDlFwpbMYJw5wHGM9 Zv1133w0msyiRdOC4UM+mpOWfXZGcZoHwdCuopkWHw== X-Google-Smtp-Source: AGHT+IHQpIkVGJCpLoc6EVwkHmU30myi6H5tPqvw7/ES4Tv2xk+lOzhBDTrqSvlq0+rORSxW/TrAAv6EHgdiBENNGNI= X-Received: by 2002:a05:6102:188c:b0:464:498f:3b6 with SMTP id ji12-20020a056102188c00b00464498f03b6mr4870448vsb.22.1702486274436; Wed, 13 Dec 2023 08:51:14 -0800 (PST) MIME-Version: 1.0 References: <432a89fafce11244287c8af757e73a2eb22a5354.1702339432.git.andreyknvl@google.com> In-Reply-To: From: Marco Elver Date: Wed, 13 Dec 2023 17:50:36 +0100 Message-ID: Subject: Re: [PATCH mm 2/4] kasan: handle concurrent kasan_record_aux_stack calls To: Andrey Konovalov 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: 94B36140023 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: jhheoutx4bio51ofzs6pox456dpzij5o X-HE-Tag: 1702486275-863656 X-HE-Meta: U2FsdGVkX19NPiPqicc0i+tgjlEpEEk9elDI1OU6bwgMsMITRgRG0ozWaSJGtHSnFmCOT8OeIJv425TawV2o8XdvDEk/aOh1QTCwp9y0x/nXJNjDmGuOT9oqeY7+seeAXtvECx6gMoEN9qQ7I0f03oRTkEaWawIfCzMLPV8rn9YBdYZWbyd+l4yoOBxAXF0ujk3eFbSMJOS/Ax9KqRag9gHiauId0GmZNIv+xgbR1F7cq1nYvMKGjFpgbgTNkMQfT/zo4EHgsxCMKLui7CEm9+SAlD1tcQ/uojcFy9DevkwU1UmXvd38TsR2AwcOzs8isQhWvyK6zOeXuAGN21SCozpUkz2jW+/dRx8pf4H75bZF6dN1oew4SHmRgVAWuuGLMlUqCVNNqUfNMME87v47iXbFOxEBKi9kz6kPGqnS/qxKVrAP5N3SWLPZF/E7Mnxyfhf2XosQTsgi7krvGfhNGF5VvmJq/U+lgq2kjC49+bd1ZzH30SazeW0tXhRfyYuvE6Dn0274w29e3aKpXHmYn2gJQIHzjCwFVV48OZIaXFGDjRlSw8DtuspRgG+PV+GhQO3xjgozCibjDB4hzi1wu0JVpJUC2MIi6/MPskHvsOhwjtFmdW5T/FaQc36zNWMW7EXSGANj6QW873OlZygtF3pIjS3Y5r0ivXYpSlr41nc3nslRznxqEn8XO8om2EB53dj3MJF9AuJ7/fy8ZcFo489ib8GFkK36tWMw9g4dJcqfGmymVbFvgC7ydoIpGc0ifeDuVYK0whGfpzcGaMTasXWPfE4/NH0AGcr+CtzirlLnTZ0YokOHQHHwABctFwcve67tmdjvXOdw4Vvomm2Yvx9MKORPUYFhHnzI3piNPeO0EJoMv083EjLlxodJ3jemblZXghZVtPpzBEa4WO33pBRsIeRjPA8si5cr0NZC5abv+ubDIko+bI7frGCtoI3t4NWiAY3JDkIAQxJSVne Um+UpaTz DsP3/w48rvODgG7DfVe8aq3wLDgdrflJB2kD9d+drhPHzanN7jbPqQge+Pg6M/1QQhgaeicAxayhA2woOkaYZpSyFSjk8WGnOoKbFY7cq7yR5wMKN/GjCJ/M3kpFc5moiVB9OB7AuPLUVVYAPB/n0GPWlRBLotODoPm3D6sihDPWQcNa7ajHD2A+a4z1Vq7/dYJih6EQ1e91xEe9n/NEsI1VxiY6XPv18C/8R3+NpMWEnzMqXBKfFXlEAXzzjX+hcrzQKhxUpmR4KWtHKMTVQGbgV8PH4BsjoTjdukMFUMY8LdnPSlFr7x10gbYqfmjtF7s6+x77pq7910MdINIZoyBx9lp70wczyWAyG/hHkmrRSOqpfXh+V5qA1QcOswKEm+xw5vRpsJt9vXEXpxURP85oH75kMRToHJMXmVPBUXzowExU= 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 Wed, 13 Dec 2023 at 15:40, Andrey Konovalov wrote= : > > On Tue, Dec 12, 2023 at 8:29=E2=80=AFPM Marco Elver wr= ote: > > > > > - 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 spin= lock. > > But we do need some kind of atomicity while rotating the aux handles > to make sure nothing gets lost. Yes, I think that'd be preferable. Although note that not all 32-bit architectures have 64-bit atomics, so that may be an issue. Another alternative is to have a spinlock next to the aux_stack (it needs to be initialized properly). It'll use up a little more space, but that's for KASAN configs only, so I think it's ok. Certainly better than a global lock.