From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from confino.investici.org (confino.investici.org [93.190.126.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2ECF63B8135 for ; Fri, 17 Jul 2026 23:14:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=93.190.126.19 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784330075; cv=none; b=WEttu9S9L6R7lXjnZ27saKfMRReZzmRCkYrhNVtj/GH8gEIclDvhmSLhBs24Ja9bXxvL5S6MovC6M+Gkyekf3a+o/Zysi92d29Z+8CjfSS/cn0ORgS63SLAfXPYhXnGzOYtNv8GORBSl6HW7AZgxT0FBv5I6+4b8Jz3Mc/Ha3Nc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784330075; c=relaxed/simple; bh=HGwdkgs/FYFqlzVnMUSCfJjWnKerJ6m8rSso7cDM8xc=; h=Date:From:To:CC:Subject:In-Reply-To:Message-ID:MIME-Version: Content-Type; b=hHGtkCxeajqbvcOVKmwRwyl1SYfhuAnnKjG51bIhgzJI9GEXF+8VX145QxksQ1X9H4fKDREYX4qfJl5klgTDtI2FEjOCiSaxIHFmLEogFmZolGW2XY1maS2Vph/ZsuAvp6349bXODcqTuvQsaMwRra4dfsLTQVIUz9EwV3dSZLs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=grrlz.net; spf=pass smtp.mailfrom=grrlz.net; dkim=pass (1024-bit key) header.d=grrlz.net header.i=@grrlz.net header.b=EVkUaYYO; arc=none smtp.client-ip=93.190.126.19 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=grrlz.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=grrlz.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=grrlz.net header.i=@grrlz.net header.b="EVkUaYYO" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=grrlz.net; s=stigmate; t=1784330061; bh=OjPPDmXMC/HSU/MW+gwhoQfoRUsOgk6RJLNyNcD/d74=; h=Date:From:To:CC:Subject:In-Reply-To:From; b=EVkUaYYO+S6TpmLYyF4gyP5wp7eC+l1x7LqBGnT8/qVHLwSi9GgTUyyJ/M6MbOUFJ j4DotJuvUOcieV+CFDYRmRk+QomLdGub6kCoZNUUz5ILkfGDaAgdnizL4fSL7hKwmJ LBwDj4ia86/4j/W6VDLt80SQ8dcvBro7dQ6WOQh4= Received: from mx1.investici.org (unknown [127.0.0.1]) by confino.investici.org (Postfix) with ESMTP id 4h25Md2hWCz112Y; Fri, 17 Jul 2026 23:14:21 +0000 (UTC) Received: by mx1.investici.org (Postfix) id 4h25Md02ZNz112R; Fri, 17 Jul 2026 23:14:20 +0000 (UTC) Date: Sat, 18 Jul 2026 00:14:20 +0100 From: Bradley Morgan To: mjguzik@gmail.com CC: AutonomousCodeSecurity@microsoft.com, avagin@gmail.com, blbllhy@gmail.com, brauner@kernel.org, jack@suse.cz, kys@microsoft.com, linux-kernel@vger.kernel.org, oleg@redhat.com, ptikhomirov@virtuozzo.com, tgopinath@linux.microsoft.com Subject: Re: [PATCH] pid: fix cad_pid use-after-free race in proc_do_cad_pid() In-Reply-To: Message-ID: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hi Cen, Applies to master, context matches. The race is real.. [re: what the synchronize_rcu() is accomplishing] put_pid() frees synchronously via kmem_cache_free(), and pid_cachep is not SLAB_TYPESAFE_BY_RCU. So the writer has to wait out RCU readers before the free, and synchronize_rcu() is that wait. Its standing in for the call_rcu() we cant use, free_pid() owns pid->rcu. Correct, just not like very obvious. please put a one line comment above it saying exactly that. > + rcu_read_lock(); > tmp_pid = pid_vnr(cad_pid); > + rcu_read_unlock(); plain load. Use rcu_dereference() and annotate cad_pid __rcu, otherwise sparse cant see any of this. > - put_pid(xchg(&cad_pid, new_pid)); > + old_pid = xchg(&cad_pid, new_pid); > + synchronize_rcu(); > + put_pid(old_pid); ok, fine. xchg() is fully ordered, one writer per old_pid, no double put. but this fixes one of two readers. kill_cad_pid() in sched/signal.h has the same unprotected load and is reachable from ctrl_alt_del(), thats hardirq context, so your grace period does nothing for it. Deinline it into kernel/pid.c and do e.g: rcu_read_lock(); pid = get_pid(rcu_dereference(cad_pid)); rcu_read_unlock(); ret = kill_pid(pid, sig, priv); put_pid(pid); Please make it a two patch series, deinline with no functional change, then fix both readers. Nits: The Fixes: tag LGTM, before 9ec52099e4b8 there was no struct to free. If you want CC stable, add a line in the changelog about impact: iirc it is root only (the sysctl is 0600) and only when cad_pid holds the last ref of an exited task. Subject prefix should be "pid:". Keep the vN changelog below the ---. with those addressed, please add: Reviewed-by: Bradley Morgan Thanks!