From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f74.google.com (mail-wm1-f74.google.com [209.85.128.74]) (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 A678C2EBDE0 for ; Fri, 5 Dec 2025 14:19:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.74 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764944373; cv=none; b=fsTgduy0j0tz9p12ErHEJuRV0JZtaTFEqXm8LCp8tufPDMfwG5aBh/nLaL4TcLIMpKUbBLi1czpoEnBr/0OTlv6Gj7M+wkE8WMpZqZdVYDfyll4JSUcg0Yg54gPk+iOon2O7d9q2ovFAfqiy1RYPMeChb4RWEjBZ30Sj5nbSvFI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764944373; c=relaxed/simple; bh=emzIcLUu0cGaXtsYDSvbR/o80gfqNGaw2w6HNBlxPgQ=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=Ae7oFNOFolzKdJsMNPpD8ZKs14UKyaBfnAjNhot0RRZ8xYOjhsuj0m1wDrKQk4RML1qKYRVgkCiFuPVZJwqGc1swTvx2JhawzRY0rjPOAZODw+dG/DvflbateeCoowpBrmuIs3W9j6L3X0THyZ1fUbYuTDbWICMiAHelWB3YdhE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=FCClnvMr; arc=none smtp.client-ip=209.85.128.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="FCClnvMr" Received: by mail-wm1-f74.google.com with SMTP id 5b1f17b1804b1-477a11d9f89so9842055e9.3 for ; Fri, 05 Dec 2025 06:19:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1764944369; x=1765549169; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Wybqj2YM/RmGp6K+DACfRxN6tMdEBqPyADU71HzzJms=; b=FCClnvMrr07nP4T+/RVbftGyb4X90u1vaNELzmN5J4nKpUB5NTAU0QOzYlm+HfQVgE NF6gJYRvc/3m60+AniCaJ/s6uMPCWEttyswe0Om6H5varlrqGwj+4fSUmix82G9/7WMR 0GDwiYm+3MCKvG60JgOI3C1uTUhBdBrJr3VRjtk/yYpbP5j3F+OMSskoSXgeOHwoZC03 1JeH/1S0mmQQVBok/3CQ5bjMhwgJAsKzWYVoFdqCfSMuk9QN8Qj44RS1cMoWloyK2XI/ MfXq8uBVj4r7KNTwoQr66E2cPrC6L6xtdy6inrNcw6OsdZTD1SzOs8AdP7I5RIxABalQ FoBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764944369; x=1765549169; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Wybqj2YM/RmGp6K+DACfRxN6tMdEBqPyADU71HzzJms=; b=DFvIY4NfGzWNBmJ5UbtigCJCGR1eC2mQa5zavIF6+BPb2UUS0RsVYnjr0/P8WOxiNq cHLlZlDmTYuJjOUS3eVQoxoBw5iIM2cdI3uUN7V0p2OcU44MbW7j9+Sk2cOVo4q6kDsr U9C26FN6n/dYxIptEDPl2g3uO7uiCLKNicva7D/yCJLKyXduQoU+MjbPASA1fJgN8o8Z KskEJpQPKeLjmPGHWIMM1EzPNzoRIxq4BkLjbv3UBQ+PynfbV9ww1SMvX3XZBPTyeQ79 ZWcSZby9va8K1rOv0frB2yTjf4eie9p3RBMKEV7DLAOowozqbBHVX8d8jLo7Jil/sBim 0klA== X-Forwarded-Encrypted: i=1; AJvYcCUX+wJrpcMcMsm2Dn7NE8icN6NoUOEivH96US08NRZNxeKIZpwWMcuhjTdPVInCa0vAjPJSaorM/zDwSIg=@vger.kernel.org X-Gm-Message-State: AOJu0Yyu/xwkoRKILgp1yUCa45fGPoylrz9rWJ3yYbSAGMv9z1MmofgG HCM9Zcz/OPktuGyBdlOazzMShJzBg23s5aVD5aSZJ4MD6H7ejCc5BZvjz8JtOewHF2Os5y3Kncy QwCLAHmb/zyBAym6W5A== X-Google-Smtp-Source: AGHT+IE2G3uXl2LwX23LIlmQwFS8l3svvl1S3Kmikb7WxrYY7e4wUY/ghc5JChAHsYEsJn7prxOterQc0vftIzg= X-Received: from wmbh4.prod.google.com ([2002:a05:600c:a104:b0:477:81d8:54c6]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:4689:b0:477:73e9:dbe7 with SMTP id 5b1f17b1804b1-4792af607f4mr116442435e9.35.1764944368528; Fri, 05 Dec 2025 06:19:28 -0800 (PST) Date: Fri, 5 Dec 2025 14:19:27 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: Message-ID: Subject: Re: rust: wrong SAFETY comments in group_leader() and pid() + questions From: Alice Ryhl To: Oleg Nesterov , Christian Brauner Cc: Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , "=?utf-8?B?QmrDtnJu?= Roy Baron" , Benno Lossin , Andreas Hindborg , Trevor Gross , Danilo Krummrich , Panagiotis Foliadis , Shankari Anand , FUJITA Tomonori , Alexey Gladkov , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Thanks a lot for your email! +Christian Brauner On Fri, Dec 05, 2025 at 03:08:23PM +0100, Oleg Nesterov wrote: > From rust/kernel/task.rs: > > pub fn group_leader(&self) -> &Task { > // SAFETY: The group leader of a task never changes after initialization, so reading this > // field is not a data race. > let ptr = unsafe { *ptr::addr_of!((*self.as_ptr()).group_leader) }; > > // SAFETY: The lifetime of the returned task reference is tied to the lifetime of `self`, > // and given that a task has a reference to its group leader, we know it must be valid for > // the lifetime of the returned task reference. > unsafe { &*ptr.cast() } > } > > /// Returns the PID of the given task. > pub fn pid(&self) -> Pid { > // SAFETY: The pid of a task never changes after initialization, so reading this field is > // not a data race. > unsafe { *ptr::addr_of!((*self.as_ptr()).pid) } > } > > The comments look wrong. Unless same_thread_group(current, task) == T, task->group_leader > and/or task->pid can change if a non-leader task's sub-thread execs. This also means that > in general it is not safe to dereference group_leader, for example this C code is not safe: > > rcu_read_lock(); > task = find_task_by_vpid(vpid); > if (task) > get_task_struct(task); > rcu_read_unlock(); > > if (task) > pid = task->group_leader->pid; // BUG! ->group_leader can be already freed > > > Now the questions. Sorry! I don't know rust. > > 1. Can I simply remove these misleading comments? Or SAFETY comment is mandatory? If the safety comments are wrong, then the code is probably wrong too! What is the correct way to read the pid or group_leader fields of a struct task_struct? > 2. I am working on the patch(es) which move ->group_leader from task_struct to > signal_struct, so the 1st change adds the new trivial helper in preparation: > > struct task_struct *task_group_leader(struct task_struct *task) > { > return task->group_leader; // will be updated > } > > Now, how can I change group_leader() to use it? I guess I need to add > > struct task_struct *rust_helper_task_group_leader(struct task_struct *task) > { > return task_group_leader(task); > } > > into rust/helpers/task.c, but will something like > > pub fn group_leader(&self) -> &Task { > unsafe { bindings::task_group_leader(self.as_ptr()) } > } > > work? I'm afraid it won't ;) That looks like it should work. The rust_helper_ function is only required if task_group_leader is marked `static inline`. Otherwise bindings:: will pick up the function straight from the C header. (As long as the relevant header is included in bindings_helper.h) That still does raise the question of how you correctly call this function if the group leader could be freed at any time. Alice