All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Maurer <mmaurer@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	Miguel Ojeda <ojeda@kernel.org>,  Gary Guo <gary@garyguo.net>,
	Luis Chamberlain <mcgrof@kernel.org>,
	 Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	linuxppc-dev@lists.ozlabs.org,  linux-kernel@vger.kernel.org,
	linux-modules@vger.kernel.org,  linux-kbuild@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	 Laura Abbott <laura@labbott.name>
Subject: Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
Date: Wed, 22 Nov 2023 13:04:09 -0800	[thread overview]
Message-ID: <CAGSQo00hyCTVsqHtrzKBBPvuH38z5yRm_4jzdi00C0RV+8APwQ@mail.gmail.com> (raw)
In-Reply-To: <CAK7LNAQt8fy5+vSwpd1aXfzjzeZ5hiyW7EW9SW7pbG2eTJZAOA@mail.gmail.com>

> So, even if you enable CONFIG_MODVERSIONS,
> nothing is checked for Rust.
> Genksyms computes a CRC from "int foo", and
> the module subsystem confirms it is a "int"
> variable.
>
> We know this check always succeeds.
>
> Why is this useful?
The reason this is immediately useful is that it allows us to have Rust
in use with a kernel where C modules are able to benefit from MODVERSIONS
checking. The check would effectively be a no-op for now, as you have correctly
determined, but we could refine it to make it more restrictive later.
Since the
existing C approach errs on the side of "it could work" rather than "it will
work", I thought being more permissive was the correct initial solution.

If we want to err on the other side (modversions passes, so we're pretty sure
it will work), I could add to the last patch support for using .rmeta files as
the CRC source for Rust symbols. This would essentially say that the interface
for the entire compilation unit has to stay the same rather than just that one
function. We could potentially loosen this requirement in the future.

With regards to future directions that likely won't work for loosening it:
Unfortunately, the .rmeta format itself is not stable, so I wouldn't want to
teach genksyms to open it up and split out the pieces for specific functions.
Extending genksyms to parse Rust would also not solve the situation -
layouts are allowed to differ across compiler versions or even (in rare
cases) seemingly unrelated code changes.

Future directions that might work for loosening it:
* Generating crcs from debuginfo + compiler + flags
* Adding a feature to the rust compiler to dump this information. This
is likely to
  get pushback because Rust's current stance is that there is no ability to load
  object code built against a different library.

Would setting up Rust symbols so that they have a crc built out of .rmeta be
sufficient for you to consider this useful? If not, can you help me understand
what level of precision would be required?

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Maurer <mmaurer@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nicolas Schier <nicolas@fjasle.eu>,
	rust-for-linux@vger.kernel.org, linux-kbuild@vger.kernel.org,
	Nick Desaulniers <ndesaulniers@google.com>,
	linux-kernel@vger.kernel.org,
	Nathan Chancellor <nathan@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>, Gary Guo <gary@garyguo.net>,
	Miguel Ojeda <ojeda@kernel.org>,
	Laura Abbott <laura@labbott.name>,
	linuxppc-dev@lists.ozlabs.org, linux-modules@vger.kernel.org
Subject: Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
Date: Wed, 22 Nov 2023 13:04:09 -0800	[thread overview]
Message-ID: <CAGSQo00hyCTVsqHtrzKBBPvuH38z5yRm_4jzdi00C0RV+8APwQ@mail.gmail.com> (raw)
In-Reply-To: <CAK7LNAQt8fy5+vSwpd1aXfzjzeZ5hiyW7EW9SW7pbG2eTJZAOA@mail.gmail.com>

> So, even if you enable CONFIG_MODVERSIONS,
> nothing is checked for Rust.
> Genksyms computes a CRC from "int foo", and
> the module subsystem confirms it is a "int"
> variable.
>
> We know this check always succeeds.
>
> Why is this useful?
The reason this is immediately useful is that it allows us to have Rust
in use with a kernel where C modules are able to benefit from MODVERSIONS
checking. The check would effectively be a no-op for now, as you have correctly
determined, but we could refine it to make it more restrictive later.
Since the
existing C approach errs on the side of "it could work" rather than "it will
work", I thought being more permissive was the correct initial solution.

If we want to err on the other side (modversions passes, so we're pretty sure
it will work), I could add to the last patch support for using .rmeta files as
the CRC source for Rust symbols. This would essentially say that the interface
for the entire compilation unit has to stay the same rather than just that one
function. We could potentially loosen this requirement in the future.

With regards to future directions that likely won't work for loosening it:
Unfortunately, the .rmeta format itself is not stable, so I wouldn't want to
teach genksyms to open it up and split out the pieces for specific functions.
Extending genksyms to parse Rust would also not solve the situation -
layouts are allowed to differ across compiler versions or even (in rare
cases) seemingly unrelated code changes.

Future directions that might work for loosening it:
* Generating crcs from debuginfo + compiler + flags
* Adding a feature to the rust compiler to dump this information. This
is likely to
  get pushback because Rust's current stance is that there is no ability to load
  object code built against a different library.

Would setting up Rust symbols so that they have a crc built out of .rmeta be
sufficient for you to consider this useful? If not, can you help me understand
what level of precision would be required?

  reply	other threads:[~2023-11-22 21:04 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-18  2:54 [PATCH v2 0/5] MODVERSIONS + RUST Redux Matthew Maurer
2023-11-18  2:54 ` Matthew Maurer
2023-11-18  2:54 ` [PATCH v2 1/5] export_report: Rehabilitate script Matthew Maurer
2023-11-18  2:54   ` Matthew Maurer
2023-11-18 11:35   ` Greg KH
2023-11-18 11:35     ` Greg KH
2023-11-18  2:54 ` [PATCH v2 2/5] modules: Refactor + kdoc elf_validity_cached_copy Matthew Maurer
2023-11-18  2:54   ` Matthew Maurer
2023-11-18 11:36   ` Greg KH
2023-11-18 11:36     ` Greg KH
2023-11-18  2:54 ` [PATCH v2 3/5] modpost: Extended modversion support Matthew Maurer
2023-11-18  2:54   ` Matthew Maurer
2023-11-18 13:42   ` kernel test robot
2023-11-18 13:42     ` kernel test robot
2023-11-18  2:54 ` [PATCH v2 4/5] rust: Allow MODVERSIONS Matthew Maurer
2023-11-18  2:54   ` Matthew Maurer
2023-11-18  2:54 ` [PATCH v2 5/5] export_report: Use new version info format Matthew Maurer
2023-11-18  2:54   ` Matthew Maurer
2023-11-22 15:49 ` [PATCH v2 0/5] MODVERSIONS + RUST Redux Masahiro Yamada
2023-11-22 15:49   ` Masahiro Yamada
2023-11-22 21:04   ` Matthew Maurer [this message]
2023-11-22 21:04     ` Matthew Maurer
2023-11-23  9:05     ` Greg KH
2023-11-23  9:05       ` Greg KH
2023-11-23 11:38       ` Masahiro Yamada
2023-11-23 11:38         ` Masahiro Yamada
2023-11-23 12:12         ` Greg KH
2023-11-23 12:12           ` Greg KH
2023-11-27 19:27           ` Matthew Maurer
2023-11-27 19:27             ` Matthew Maurer
2023-11-28  8:05             ` Greg KH
2023-11-28  8:05               ` Greg KH
2023-11-28  8:44               ` Greg KH
2023-11-28  8:44                 ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGSQo00hyCTVsqHtrzKBBPvuH38z5yRm_4jzdi00C0RV+8APwQ@mail.gmail.com \
    --to=mmaurer@google.com \
    --cc=gary@garyguo.net \
    --cc=laura@labbott.name \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=masahiroy@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.