From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 5555ADF42; Wed, 2 Jul 2025 10:01:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751450500; cv=none; b=kSjGOx1Rz/EgTiQ8ltS4OGvg3GC2rPHeZVPDw9emf+XcnhkHOLwHPOcSYinNAq3jmGsfRx1b3rgahnl5F3vN7DPYOn51grUNd88CxNhJGLftSl43BfrYf1Qn0g/84ZQwcLElujPkn7Mej4oa0TDm8i+5gwM5P9lPmc8j2DEgarc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751450500; c=relaxed/simple; bh=I/1I5NyRVzSpJe49DY0iJKRA4CT2xnDLtwSt04yjusA=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=VK7mnxWldztCoe3mGUZ+4Ma69L+b5DbHx1Qz/qxcuFLleFBtCTEnaWpOH2+oFbxpOgak6mspkBREOhmm0O79/9awSXvQkJHc8fhh68jlhdAa4MWlYR8MrPzkGzioqn2EziOEh2TDKihjgiNE9aQ0btq7RgfaJqyaxWejya35rL0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GnhKpsA/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GnhKpsA/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EB399C4CEED; Wed, 2 Jul 2025 10:01:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1751450498; bh=I/1I5NyRVzSpJe49DY0iJKRA4CT2xnDLtwSt04yjusA=; h=Date:Cc:Subject:From:To:References:In-Reply-To:From; b=GnhKpsA/lnIUGyA+lLBBgIDrCMjgNNKFl5jp0Mr6k4lw+b+ScMzQi3C321v1tleTo UXfGXroSQ7z7dQrIqMjw/NCxtx+MAINjdxp0IKefYMgggalereJ/DOcWEuoi7x2CDT 2ZucnXVotdMTzAnWus5/2pe956AoBFyp+6K3X6uCcC7ktoedAPMQ4zV8WdxHz+H1Ts H0xCijMSecRcVAJQLbIOoHP9WJgxOXGJbd+ua85Zuh7c7cUYKRqbh/9vmdLeDl8LIE +ZKwEuxNp1n5NNUEXdTgbGlbe7SJ73HhrXii0ryBAT5iicRWTcQDGzRmt5VobbWTm9 gN0hKbl5kxnPA== Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 02 Jul 2025 12:01:28 +0200 Message-Id: Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Alice Ryhl" , "Masahiro Yamada" , "Nathan Chancellor" , "Luis Chamberlain" , "Danilo Krummrich" , "Nicolas Schier" , "Trevor Gross" , "Adam Bratschi-Kaye" , , , , "Petr Pavlu" , "Sami Tolvanen" , "Daniel Gomez" , "Simona Vetter" , "Greg KH" , "Fiona Behrens" , "Daniel Almeida" , Subject: Re: [PATCH v13 2/6] rust: introduce module_param module From: "Benno Lossin" To: "Andreas Hindborg" , "Miguel Ojeda" X-Mailer: aerc 0.20.1 References: <20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org> <87v7om4jhq.fsf@kernel.org> <878qlh4aj1.fsf@kernel.org> <87plepzke5.fsf@kernel.org> <87wm8txysl.fsf@kernel.org> <9G3W1seaM7elcwWXaeoaa2nfpFYCf-AmBdvZhACGP13KGUtTPVMwGNYdTQsdtp8ru7GIP3-UYTzXscC1MRUKrg==@protonmail.internalid> <87h5zxxtdw.fsf@kernel.org> <87bjq4xpv7.fsf@kernel.org> <87zfdovvz4.fsf@kernel.org> <87o6u3vw04.fsf@kernel.org> In-Reply-To: <87o6u3vw04.fsf@kernel.org> On Wed Jul 2, 2025 at 10:26 AM CEST, Andreas Hindborg wrote: > "Miguel Ojeda" writes: > >> On Tue, Jul 1, 2025 at 5:43=E2=80=AFPM Benno Lossin = wrote: >>> >>> Ultimately this is something for Miguel to decide. >> >> Only if you all cannot get to an agreement ;) > >> >> If Andreas wants to have it already added, then I would say just mark >> it `unsafe` as Benno recommends (possibly with an overbearing >> precondition), given it has proven subtle/forgettable enough and that, >> if I understand correctly, it would actually become unsafe if someone >> "just" added "reasonably-looking code" elsewhere. > > You are right that if someone added code to the API, the API could > become unsound. But that is the deal with all our APIs and I don't agree > that the details are very subtle here. Someone would need to add sysfs > support or user provided parameter parsing to cause the unsoundness we > are talking about. Normally the safety requirements & invariants are *local*. We do have some global ones, but this one would be another one. And it's not easy to control IMO (no code is allowed to run before parameter parsing finished!). > Anyone attempting such a task should have proper understanding of the > code first, and given the ample amount of `NOTE` comments I have added, > it should be clear that the concurrent accesses that this addition would > introduce, needs to be accounted for, to avoid data races. Well if I add a way to add a task to a work queue before parameter parsing, I don't need to touch this file or even know about it. > I will add myself as a reviewer for the rust module parameter parsing > code if that is OK with module maintainers. I think it's a good idea, but it is orthogonal and doesn't address the issue, since any tree can merge code that breaks the invariant above. >> That way we have an incentive to make it safe later on and, more >> importantly, to think again about it when such a patch lands, >> justifying it properly. And it could plausibly protect out-of-tree >> users, too. > > Again, I do not think it is reasonable to mark this function unsafe. We mark it `unsafe` only until atomics are merged and then we make it safe. You proposed to do it the other way and make it safe, though possibly unsound when someone adds code breaking the invariant and making it fully sound later. I don't think we should have global invariants that are temporary. --- Cheers, Benno