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 24067C35FFC for ; Sat, 22 Mar 2025 10:16:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0C2E1280002; Sat, 22 Mar 2025 06:15:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 04DE1280001; Sat, 22 Mar 2025 06:15:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE214280002; Sat, 22 Mar 2025 06:15:58 -0400 (EDT) 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 C142E280001 for ; Sat, 22 Mar 2025 06:15:58 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 6FDC51A0CAF for ; Sat, 22 Mar 2025 10:15:58 +0000 (UTC) X-FDA: 83248781196.20.9E8A6AD Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by imf15.hostedemail.com (Postfix) with ESMTP id 7AA97A0009 for ; Sat, 22 Mar 2025 10:15:56 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=T8NAizK3; spf=pass (imf15.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.128.54 as permitted sender) smtp.mailfrom=mjguzik@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=1742638556; 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=CqVkROA4dG75aQjTk+R2qOiPfKxllk/0j3YoZoCJgBI=; b=frN1FvTAn07f0HJSQJjJZ/2t/lw5IWDzEw+JNZKtxHSGuOkwQ5xw6K55GKkbe+N1QDLi// wzdSqMUIWCu9qvxweHGovd0GfO04Z9Knue1qUZEFTv4kTxEEroy74xjEv0jEIBFnh06R0W UgfxSi57RzZIudI8/aw302J32dTRV3g= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742638556; a=rsa-sha256; cv=none; b=DiEGKLuzYqQlqbDSGZE0I3rwjBG2GjnbWOtvFwCX8KEiqP7I8vRYLu7/O7DLsLZwTS3TdP l8jbQGP/LA2SJWqKVMZEf9ePmzrBr/GSJOuA9zcvKzmVepM431Fviqtfbw6dH4J5lOrLG/ 5Eke4ejN1ZFWr97dX/TDTvVpDBG96eE= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=T8NAizK3; spf=pass (imf15.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.128.54 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-4394036c0efso16665385e9.2 for ; Sat, 22 Mar 2025 03:15:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1742638555; x=1743243355; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=CqVkROA4dG75aQjTk+R2qOiPfKxllk/0j3YoZoCJgBI=; b=T8NAizK3cYEHsrnpQ3Js2AZH0tUKXMPfZGDqIMX8oKmt1LT2dkBPoyF6IGrQLmYYVE erAIrRoX/d6zGiyCbR3Wo7Cles0L8r1QUur3SF4kz6J1xmfZ1q2Ibwr6SygxWhFQowzf UN0X0AT31kUP2OqcPkMAlByGkaqtTXTw+Cij6V3XXuyhZR5BEMG8qriSlRX+QaLkavLs 4g7MO1vxO9j78nZPHHw0pnI1mU15G/XK7cCEiWV2Sqe4wkpTiygdwb1Y54rwUudaqwYh KgluMdVLnbgDTG/xBRWi2ONSZSug1owelXXZkiVjqfO44AAbNKuyeXT4YkubaJSTDLXQ rmHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742638555; x=1743243355; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=CqVkROA4dG75aQjTk+R2qOiPfKxllk/0j3YoZoCJgBI=; b=Bvkgo4JywtxQvWe2uygpa/1Q+fLIeqdhUWAhsRQdgPQ+VHaTdz2r8q7ewpx7byeemA NuQwRPt9UAv+clAiHD80D0gl83y4UZSd3KvjPULgbcPtIhO/ksjr1jf+V3UvOOXmI+Oc sCnMu/EFO3pK/WnGvetvkGO1CxLgGpWRHmEAKTL60EyHz9x3qjNAQMY2u8ZG9fRwgcgx BTxtiO/9pYbKpP8Qnf8aO7qeVCTfBN7gaEJQz83WadluP0Ny/xM2UFPY8LTUbiUtWmF9 POtG4z7fRjq6xuDUh+Pm7wYqcdwZqSB291vfGxpbjLpIThNWlptiPn3UwTnjDG9du+lO QFXw== X-Forwarded-Encrypted: i=1; AJvYcCUBQuzfhyloe1XhDHk95Uo63S02qugAyGkwW5A1TdcauhI0wRC/84JY3uYDh7ZwU07IbpKN7NOFIA==@kvack.org X-Gm-Message-State: AOJu0Yxfabig5PAWKBeFKaDw07U8vOStA8VaZ4+/zgTGKDlal124cCGw 2gVBxr9d2OEr/FRJ4deQEvdJfUCEp5168JVSrIIEybcjkilz/ZU0 X-Gm-Gg: ASbGnctrSwCnj4S0nZAOXdw8Ec5Sz7e8m5ZXetIpfwlNCb0dk03QcNXzmE125nea/Br d2R7n1746tRczXv3GG5q19iEJWCouA3EZ6uYzqZ+3Dy7lD2WSPkuxKST9r8elEbKfzpqv2iMErS 5kapIPE19KiSh1dX/QKtdyVUj6d+KjzjaZ7cG5bqyK0luGMBQ69YO45gpT+BfV4ojHlUaCkpYII BfNBhvWYQIHE7D6es0bHlrCtB0gW5GZT9AUwK/pnREGa60AVzg/bWwiJ099DJBFUtbRu6lE8gHE tp/56JwGh8VW+zCZFY9TcShk9z/3tvvtsKSxW2RZopizxGOgbFcmGoiFpsWY X-Google-Smtp-Source: AGHT+IEIScTNuVLx6DrGfe/RJueop+NPQUZ6yI/awEzqGTgq5nw4AOwZc7P71l6R83PgdI8tK/lFGw== X-Received: by 2002:a05:600c:3ca5:b0:43d:ed:acd5 with SMTP id 5b1f17b1804b1-43d509edc2emr62650215e9.10.1742638554556; Sat, 22 Mar 2025 03:15:54 -0700 (PDT) Received: from f (cst-prg-82-124.cust.vodafone.cz. [46.135.82.124]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43d4fd9decbsm52659575e9.27.2025.03.22.03.15.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Mar 2025 03:15:53 -0700 (PDT) Date: Sat, 22 Mar 2025 11:15:44 +0100 From: Mateusz Guzik To: Kees Cook Cc: Al Viro , Christian Brauner , Oleg Nesterov , jack@suse.cz, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, syzkaller-bugs@googlegroups.com, syzbot Subject: Re: [syzbot] [fs?] [mm?] KCSAN: data-race in bprm_execve / copy_fs (4) Message-ID: References: <67dc67f0.050a0220.25ae54.001f.GAE@google.com> <202503201225.92C5F5FB1@keescook> <20250321-abdecken-infomaterial-2f373f8e3b3c@brauner> <20250322010008.GG2023217@ZenIV> <202503212313.1E55652@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <202503212313.1E55652@keescook> X-Rspam-User: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 7AA97A0009 X-Stat-Signature: 4xtfzc4cssjao4xq3bqawxsq8439bbkb X-HE-Tag: 1742638556-681860 X-HE-Meta: U2FsdGVkX1/b2/MAf50gippV4CiV1USzMm/UCsY5X8t/aKWuEPj5hvS2PLFkxhyKASMrAc178p25DGdBHLelMuGXJLuWo/kpoQfbJ1AkK/oLmuLwmfhZHDRNAaA4o13WXZGIimaF7qZpZRVCswHDYq5DNQnbemefSruRkPDVa1OWlCzfVvx5lYdeX4YsN9kqT4UW0Vzpb9yAu3exzBftfGebF74Lsfoyl6BnzZ7lI5GcVSxvzRJOrXmLjcI32YSrCmrMclUEbFsR4GOBqB123oY3MNOz9PzEF7Reuq4S2HteKJQowKlwcRa2oX519yVRWKcnzsExOa7fcBf/CN4hEGIs6/bY6Sv5EmBUPoaIQBrtA7aDQyEVWy96MpErsLjUKzovx5tPJZAShZ/eFH2aiv4sk0shKmjWn7kxJ/IS6S14sZ1/KBNTrh1jFUy3TVWx3ok/cOvE1yl65qsHlYcNIA/fRIfePGsB3L2mOi0S0wonDMGrc2Ls6NhFpqx/SKcBU534LE0j7grfA8LBJjqUneqKW4CjjLItp4KwXP6cy4A1UnD0mZMFXuVZxQV/gv2LCfwWsEUFsNmWT5ZETVEtPmohDTChNv8z6iBrTN+IIvPNsQXFsu1mkMUeCZncd7QXmr3f2YGsE8SsaPFnY94Dc49txewhNQnvjDBecN/OTkX/rEQ/De9ZuLF7T8+iXRTDfm0GKW2S6MH9fAAYgybmQ7AcAC4wkbw8zFNY6j3xprqmGrHWcKXkpA3NINq2e+wlrKE3oE7V+ReciWeTfbH1YbgC2PpEmrEOobmaOI5xd6T43s6njop6YLfxFDkd/SFFHWo7u3j+OWB5qu9dfPh5ubGoxDxbhQ6Ne3Q3M4MvXCqc20MflvPRHEVXbIl7t6OVg8sQUpQI9akyF4dl0mXD3wPyPscKoSZzzvbWRceZwPC9LjtxMt+txKazYcjzuwYsipksz23LiPme5JUmpv+ tXwVcIme 4G4gyxA5U+/ikYHO+FmvGY1OYBNaPDWzA2lps358spomMSv42xo1jbwAmblYFPv6pbjxv4dygfIkZ+ilTA1ErPVPRCRhgGu1tIkIhhJWuIffPBIaKdUH4mDzwrwr1KE36v0FAxYv2OJGa1VJltPjg2KHQJeqkl4ZIXwO4q2gYvr7EFxkB/riqBwBHWsrYVDeTGGPxeO1F5sm3bComjX2A4cDpMyT/uHIFjN4dcGIvxeUpN6lMQ7PNQO86oepYW1ylCQHtRrlFQuC60GqC/swnR98uA0fRzXxRPzKdqCgT10os4vet3mouS3iGNnHQ9w25gIpx33bX8SiCFBG+wuPiwVf6Bxp0Gd1NQJm3YB1XQ2T0v2ZVR8DTxIIw5T3tsxleoQ15kODJkL+YVkj7HppKDc/dOp58nDDD7JNJTdc81qNymv7NqwnHqLREZrLhcOAtEzhnD1w2fzW1wiHPyzVB51RqZmDjXqC1ReT/pMabO0u611qzrEJnUN+FgCx9EPu2K/ySMTeiAooxfhcYEX2CxCrXtbbATM2L5xQxRXeHaJpRqEEW7Ta1cTrQfqb0a7FWZeHs1HXI3PvkzdA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000120, 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 Fri, Mar 21, 2025 at 11:26:03PM -0700, Kees Cook wrote: > On Sat, Mar 22, 2025 at 01:00:08AM +0000, Al Viro wrote: > > On Fri, Mar 21, 2025 at 09:45:39AM +0100, Christian Brauner wrote: > > > > > Afaict, the only way this data race can happen is if we jump to the > > > cleanup label and then reset current->fs->in_exec. If the execve was > > > successful there's no one to race us with CLONE_FS obviously because we > > > took down all other threads. > > > > Not really. > > Yeah, you found it. Thank you! > > > 1) A enters check_unsafe_execve(), sets ->in_exec to 1 > > 2) B enters check_unsafe_execve(), sets ->in_exec to 1 > > With 3 threads A, B, and C already running, fs->users == 3, so steps (1) > and (2) happily pass. > > > 3) A calls exec_binprm(), fails (bad binary) > > 4) A clears ->in_exec > > 5) C calls clone(2) with CLONE_FS and spawns D - ->in_exec is 0 > > D's creation bumps fs->users == 4. > > > 6) B gets through exec_binprm(), kills A and C, but not D. > > 7) B clears ->in_exec, returns > > > > Result: B and D share ->fs, B runs suid binary. > > > > Had (5) happened prior to (2), (2) wouldn't have set ->in_exec; > > had (5) happened prior to (4), clone() would've failed; had > > (5) been delayed past (6), there wouldn't have been a thread > > to call clone(). > > > > But in the window between (4) and (6), clone() doesn't see > > execve() in progress and check_unsafe_execve() has already > > been done, so it hadn't seen the extra thread. > > > > IOW, it really is racy. It's a counter, not a flag. > > Yeah, I would agree. Totally untested patch: > > diff --git a/fs/exec.c b/fs/exec.c > index 506cd411f4ac..988b8621c079 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1632,7 +1632,7 @@ static void check_unsafe_exec(struct linux_binprm *bprm) > if (p->fs->users > n_fs) > bprm->unsafe |= LSM_UNSAFE_SHARE; > else > - p->fs->in_exec = 1; > + refcount_inc(&p->fs->in_exec); > spin_unlock(&p->fs->lock); > } > > @@ -1862,7 +1862,7 @@ static int bprm_execve(struct linux_binprm *bprm) > > sched_mm_cid_after_execve(current); > /* execve succeeded */ > - current->fs->in_exec = 0; > + refcount_dec(¤t->fs->in_exec); > current->in_execve = 0; > rseq_execve(current); > user_events_execve(current); > @@ -1881,7 +1881,7 @@ static int bprm_execve(struct linux_binprm *bprm) > force_fatal_sig(SIGSEGV); > > sched_mm_cid_after_execve(current); > - current->fs->in_exec = 0; > + refcount_dec(¤t->fs->in_exec); > current->in_execve = 0; > > return retval; The bump is conditional and with this patch you may be issuing refcount_dec when you declined to refcount_inc. A special case where there are others to worry about and which proceeds with an exec without leaving in any indicators is imo sketchy. I would argue it would make the most sense to serialize these execs. Vast majority of programs are single-threaded when they exec with an unshared ->fs, so they don't need to bear any overhead nor complexity modulo a branch. For any fucky case you can park yourself waiting for any pending exec to finish.