From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E9477411671 for ; Mon, 20 Jul 2026 16:05:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784563517; cv=none; b=s4ZG6HnLntoUGMw4hSWpOh+EgBEZYHixf4BxtW65six/C2/FG8+jP/NLqs5b6YHDb+gSr4nbCzjV/YIN5tjjT8ZbWusGBMjMW0wXIe6uEDY1vS3sxkxx1V0rZnFGnrFuJXxHLQ+0fE/rcU4bhEciqkh7QlnqeOH/TcNpigwrWpM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784563517; c=relaxed/simple; bh=v8ht1d3qnpQXprqhvwRtoKvdsC7wSpNGPpNSlmD5f7w=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=Ai2vEcyWdBK3cDCNlRxPnVJ3gIPN9+489ThYne8jKt+LReQYcj0zYbjF0c1PWPkZ0PG7D9LMulQQCDWJtuFI/1I2GmTnfI8PFCcW7PG0sm2fiHXlqxjVghHBoh1l8vOA3cFIxXOl9p+8v/upY7B9zBX7wswquOKx8K2J2rbvq5c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dilger.ca; spf=pass smtp.mailfrom=dilger.ca; dkim=pass (2048-bit key) header.d=dilger-ca.20251104.gappssmtp.com header.i=@dilger-ca.20251104.gappssmtp.com header.b=fGK/9ooR; arc=none smtp.client-ip=209.85.216.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dilger.ca Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=dilger.ca Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=dilger-ca.20251104.gappssmtp.com header.i=@dilger-ca.20251104.gappssmtp.com header.b="fGK/9ooR" Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-38e88b60121so646270a91.3 for ; Mon, 20 Jul 2026 09:05:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dilger-ca.20251104.gappssmtp.com; s=20251104; t=1784563515; x=1785168315; darn=vger.kernel.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:content-type:from:to:cc :subject:date:message-id:reply-to:content-type; bh=vb1pv8tfTVgoUonZ7LoUildCsDWvCj01pODKNinxLq0=; b=fGK/9ooRTxdr3+5zvsFM6P/Jn8MbzRMnQ2SVZ0V3dpjIQwHLiR1kj9K10kf6Zm8o9e 62jvb9i6qzBfE33X6Kt+2WbjIvWr+nUYPlIf5hmyY7z+HrPW/L9yCVo7kaNUFMnMKdKy qGgbNQIs29m6P3o8YBZZqZJfcorO4gGjINIFB6cHags9kJzsvlaz1NUH28KTkdAmsLyY twZcUrsUvY98QW/Y2NauyOfz7ZZS/7kIOeKfTOD53akmCHyZC2gGwCTpXLd+kVdx3F8O qb0be7XuNVUaNN+zH37LTFz50S0RSxP2Lk5TyrC1PkL/NLXebLSt5eSSO+T/HOIoG290 q7Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784563515; x=1785168315; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:content-type:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=vb1pv8tfTVgoUonZ7LoUildCsDWvCj01pODKNinxLq0=; b=B34J9vhbmSMJLmU7SFiBseOKF7gT6QvV0RmSmIS1VQAq1S6zbUK1U+ze9S65JXLkX2 C9mGWOYqQOf3W/ZTCdGkdb5Dn/o7f6sZGvbGT/EUFveL+z5A9gglfOfwiFmOZo9KrtFl x9laYi2fh/GfLFux6OVsmuXHDXKwAG0HmiDjp9dX4hb77V0fDv/ba0Dt1+nq7XobewrF lykS7o3JcRuxl1aZTFEsjzOSeQqA8lflxePmBr+coB513RESe5bf+4XCrxJxkwzCzscL bU5GblkKa1z70YK3QVVnPmkkgEP7I9f/X9aU5A3Tr4qU91ZWfHfXkEL+0t1kBjxuOE87 iTeg== X-Forwarded-Encrypted: i=1; AHgh+RohJF+K+TagVYhBrhzbr7uuxmkQzXT1KQozVS89MXUzNsUeO4yZImLW8NTBsEYXXXTwE502C4Ssc0vdpn0=@vger.kernel.org X-Gm-Message-State: AOJu0YxGiaiRxcTTSOdzpg9MoBX4al43GRguRZx0CBFC1GCk//UYL66W qvyNpMq0Fosob2nqOo6CQqf/iFTBvN7CaTcTAFLqwJf2AI+aB8c+SGPAAx6cmu8kZko= X-Gm-Gg: AR+sD103Hfky4WaxEx47m/TJ7x8nE64Zhcw3+0oRTjhoJhHsmvEfIz5IPI1b2XMROEo fWGOV64WsgjX9+QsWW8mKecqEpbWREHshTwjOfKu1vcoAGZg/rtNyWhUgHOysn15xUfC6OlM5lN rPmI8KpWt54omQQfi4nxKLFQ//UqNIYC7QAE513CFRfLwkT++JWMuAloYWwXXM18xa2nD1orPOx vvJcimKGRyqVn4uddLQKZQOY5bWyyyiYUtrznjOaXGsqcCKYLLSMHXGQzQilWQRyCWp8haNs/KK uMS/IgmvzIwfVZ4EiMsGOQykGygNMyf4lMogL0nFz4Dyae8q1Xa/NgbQgWb6d5VWsZ2ESdY1gBL o25XLJYCG1NxvIOOz1zZ1PY//4nkxOGZ1Y0zEzdlkNll967fALWwpc9lXGCmVTE+dijMcJz9iXV taJkj2EhPTvk+yYc59izbziR+1jIPZ5gxlssYF4nje X-Received: by 2002:a17:90b:1d45:b0:38c:e9e9:e7ce with SMTP id 98e67ed59e1d1-38e4b41bfd4mr15395885a91.3.1784563514958; Mon, 20 Jul 2026 09:05:14 -0700 (PDT) Received: from smtpclient.apple ([2604:3d09:3a84:1700:d9b7:1db:a020:b64d]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-38e9208f731sm4346a91.6.2026.07.20.09.05.13 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Jul 2026 09:05:14 -0700 (PDT) Content-Type: text/plain; charset=us-ascii Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.100.1.1.5\)) Subject: Re: [PATCH v2] ext4: fix race in ext4_mb_check_group_pa From: Andreas Dilger In-Reply-To: <20260720065505.4019225-1-rafad900@gmail.com> Date: Mon, 20 Jul 2026 10:05:02 -0600 Cc: tytso@mit.edu, libaokun@linux.alibaba.com, jack@suse.cz, linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: 7bit Message-Id: <2AAF0DFB-2FD5-405B-8965-1C88AF6E2B98@dilger.ca> References: <20260720065505.4019225-1-rafad900@gmail.com> To: rafad900 X-Mailer: Apple Mail (2.3864.100.1.1.5) On Jul 20, 2026, at 00:54, rafad900 wrote: > ext4_mb_check_group_pa drops the reference count on the previous > best PA using atomic_dec(&cpa->pa_count) without holding the > cpa->pa_lock. > > This causes race with ext4_discard_preallocations() which checks > pa_count to decide whether a PA is still in use. If the pa_count > is dec between the check and the discard, the PA can be freed > while ext4_mb_check_group_pa() still holds a reference to it. Can you please explain this race condition further? I don't see where ext4_mb_check_group_pa() is using cpa after the reference is dropped. > Fix this by taking the cpa->pa_lock around the atomic_dec. > Similar to pa->pa_lock which is taken outside of the > ext4_mb_check_group_pa() function. At this point, it wouldn't be clear why `pa_count` needs to be an atomic at all, if `pa_lock` is always held during inc/dec/check? > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index ed1bd00e11cd..c9a118ae4658 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -4833,7 +4833,9 @@ ext4_mb_check_group_pa(ext4_fsblk_t goal_block, > return cpa; > > /* drop the previous reference */ > + spin_lock(&cpa->pa_lock); > atomic_dec(&cpa->pa_count); > + spin_unlock(&cpa->pa_lock); > atomic_inc(&pa->pa_count); > return pa; > } In ext4_mb_check_group_pa() there is no reference to `cpa` after the refcount is dropped. In its one caller ext4_mb_use_preallocated(): list_for_each_entry_rcu(tmp_pa, &lg->lg_prealloc_list[i], pa_node.lg_list) { spin_lock(&tmp_pa->pa_lock); if (tmp_pa->pa_deleted == 0 && tmp_pa->pa_free >= ac->ac_o_ex.fe_len) { cpa = ext4_mb_check_group_pa(goal_block, tmp_pa, cpa); } spin_unlock(&tmp_pa->pa_lock); } rcu_read_unlock(); } if (cpa) { ext4_mb_use_group_pa(ac, cpa); return true; } return false; } It *looks* like 'cpa' is used after ext4_mb_check_group_pa(), but it is replaced on the return by 'tmp_pa' in that case, so there is no further use after the refcount is dropped AFAICS. Even the list iteration is using 'tmp_pa', so that couldn't be it either. There may be a race condition somewhere, but the commit message doesn't provide clear details of what it is. Cheers, Andreas