* [GIT PULL] sysctl changes for v6.11-rc1 [not found] <CGME20240716141703eucas1p2f6ddaf91b7363dd893d37b9aa8987dc6@eucas1p2.samsung.com> @ 2024-07-16 14:16 ` Joel Granados 2024-07-16 18:13 ` Kees Cook ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Joel Granados @ 2024-07-16 14:16 UTC (permalink / raw) To: Linus Torvalds Cc: Luis Chamberlain, Jeff Johnson, Joel Granados, Kees Cook, Thomas Weißschuh, Wen Yang, linux-kernel, linux-fsdevel Linus Note: I have update the capabilities in my signing key. I don't think anything changes on your side, but thought I'd mention it for good measure. Pulling from https://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git would probably solve any unforeseen issues. The following changes since commit c3f38fa61af77b49866b006939479069cd451173: Linux 6.10-rc2 (2024-06-02 15:44:56 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/ tags/sysctl-6.11-rc1 for you to fetch changes up to acc154691fc75e1a178fc36624bdeee1420585a4: sysctl: Warn on an empty procname element (2024-06-13 10:50:52 +0200) ---------------------------------------------------------------- sysctl changes for 6.11-rc1 Summary * Remove "->procname == NULL" check when iterating through sysctl table arrays Removing sentinels in ctl_table arrays reduces the build time size and runtime memory consumed by ~64 bytes per array. With all ctl_table sentinels gone, the additional check for ->procname == NULL that worked in tandem with the ARRAY_SIZE to calculate the size of the ctl_table arrays is no longer needed and has been removed. The sysctl register functions now returns an error if a sentinel is used. * Preparation patches for sysctl constification Constifying ctl_table structs prevents the modification of proc_handler function pointers as they would reside in .rodata. The ctl_table arguments in sysctl utility functions are const qualified in preparation for a future treewide proc_handler argument constification commit. * Misc fixes Increase robustness of set_ownership by providing sane default ownership values in case the callee doesn't set them. Bound check proc_dou8vec_minmax to avoid loading buggy modules and give sysctl testing module a name to avoid compiler complaints. Testing * This got push to linux-next in v6.10-rc2, so it has had more than a month of testing ---------------------------------------------------------------- Jeff Johnson (1): sysctl: Add module description to sysctl-testing Joel Granados (8): locking: Remove superfluous sentinel element from kern_lockdep_table mm profiling: Remove superfluous sentinel element from ctl_table sysctl: Remove check for sentinel element in ctl_table arrays sysctl: Replace nr_entries with ctl_table_size in new_links sysctl: Remove superfluous empty allocations from sysctl internals sysctl: Remove "child" sysctl code comments sysctl: Remove ctl_table sentinel code comments sysctl: Warn on an empty procname element Thomas Weißschuh (3): sysctl: always initialize i_uid/i_gid utsname: constify ctl_table arguments of utility function sysctl: constify ctl_table arguments of utility function Wen Yang (1): sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array fs/proc/proc_sysctl.c | 70 ++++++++++++++++++++++++++---------------------- include/linux/sysctl.h | 2 +- kernel/locking/lockdep.c | 1 - kernel/sysctl-test.c | 50 ++++++++++++++++++++++++++++++++++ kernel/sysctl.c | 31 +++++++++------------ kernel/utsname_sysctl.c | 2 +- lib/alloc_tag.c | 1 - net/sysctl_net.c | 11 ++------ 8 files changed, 105 insertions(+), 63 deletions(-) -- Joel Granados ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-16 14:16 ` [GIT PULL] sysctl changes for v6.11-rc1 Joel Granados @ 2024-07-16 18:13 ` Kees Cook 2024-07-17 19:46 ` Joel Granados 2024-07-16 21:43 ` pr-tracker-bot 2024-08-06 18:57 ` Solar Designer 2 siblings, 1 reply; 10+ messages in thread From: Kees Cook @ 2024-07-16 18:13 UTC (permalink / raw) To: Linus Torvalds Cc: Joel Granados, Luis Chamberlain, Jeff Johnson, Thomas Weißschuh, Wen Yang, linux-kernel, linux-fsdevel On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > * Preparation patches for sysctl constification > > Constifying ctl_table structs prevents the modification of proc_handler > function pointers as they would reside in .rodata. The ctl_table arguments > in sysctl utility functions are const qualified in preparation for a future > treewide proc_handler argument constification commit. And to add some additional context and expectation setting, the mechanical treewide constification pull request is planned to be sent during this merge window once the sysctl and net trees land. Thomas Weißschuh has it at the ready. :) -- Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-16 18:13 ` Kees Cook @ 2024-07-17 19:46 ` Joel Granados 2024-07-17 20:05 ` Thomas Weißschuh 2024-07-17 22:15 ` Kees Cook 0 siblings, 2 replies; 10+ messages in thread From: Joel Granados @ 2024-07-17 19:46 UTC (permalink / raw) To: Kees Cook Cc: Linus Torvalds, Luis Chamberlain, Jeff Johnson, Thomas Weißschuh, Wen Yang, linux-kernel, linux-fsdevel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1086 bytes --] On Tue, Jul 16, 2024 at 11:13:24AM -0700, Kees Cook wrote: > On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > > * Preparation patches for sysctl constification > > > > Constifying ctl_table structs prevents the modification of proc_handler > > function pointers as they would reside in .rodata. The ctl_table arguments > > in sysctl utility functions are const qualified in preparation for a future > > treewide proc_handler argument constification commit. > > And to add some additional context and expectation setting, the mechanical > treewide constification pull request is planned to be sent during this > merge window once the sysctl and net trees land. Thomas Weißschuh has > it at the ready. :) Big fan of setting expectations :). thx for the comment. Do you (@kees/ @thomas) have any preference on how to send the treewide const patch? I have prepared wording for the pull request for when the time comes next week, but if any of you prefer to send it through another path different than sysctl, please let me know. Best -- Joel Granados ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-17 19:46 ` Joel Granados @ 2024-07-17 20:05 ` Thomas Weißschuh 2024-07-17 22:15 ` Kees Cook 1 sibling, 0 replies; 10+ messages in thread From: Thomas Weißschuh @ 2024-07-17 20:05 UTC (permalink / raw) To: Joel Granados Cc: Kees Cook, Linus Torvalds, Luis Chamberlain, Jeff Johnson, Wen Yang, linux-kernel, linux-fsdevel On 2024-07-17 21:46:20+0000, Joel Granados wrote: > On Tue, Jul 16, 2024 at 11:13:24AM -0700, Kees Cook wrote: > > On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > > > * Preparation patches for sysctl constification > > > > > > Constifying ctl_table structs prevents the modification of proc_handler > > > function pointers as they would reside in .rodata. The ctl_table arguments > > > in sysctl utility functions are const qualified in preparation for a future > > > treewide proc_handler argument constification commit. > > > > And to add some additional context and expectation setting, the mechanical > > treewide constification pull request is planned to be sent during this > > merge window once the sysctl and net trees land. Thomas Weißschuh has > > it at the ready. :) > > Big fan of setting expectations :). thx for the comment. > Do you (@kees/ @thomas) have any preference on how to send the treewide > const patch? I have prepared wording for the pull request for when the > time comes next week, but if any of you prefer to send it through > another path different than sysctl, please let me know. Sysctl sounds good to me. Linus, if you are already interested in the upcoming patch and its background: https://lore.kernel.org/lkml/20240619-sysctl-const-handler-v2-1-e36d00707097@weissschuh.net/ Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-17 19:46 ` Joel Granados 2024-07-17 20:05 ` Thomas Weißschuh @ 2024-07-17 22:15 ` Kees Cook 1 sibling, 0 replies; 10+ messages in thread From: Kees Cook @ 2024-07-17 22:15 UTC (permalink / raw) To: Joel Granados Cc: Linus Torvalds, Luis Chamberlain, Jeff Johnson, Thomas Weißschuh, Wen Yang, linux-kernel, linux-fsdevel On Wed, Jul 17, 2024 at 09:46:20PM +0200, Joel Granados wrote: > On Tue, Jul 16, 2024 at 11:13:24AM -0700, Kees Cook wrote: > > On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > > > * Preparation patches for sysctl constification > > > > > > Constifying ctl_table structs prevents the modification of proc_handler > > > function pointers as they would reside in .rodata. The ctl_table arguments > > > in sysctl utility functions are const qualified in preparation for a future > > > treewide proc_handler argument constification commit. > > > > And to add some additional context and expectation setting, the mechanical > > treewide constification pull request is planned to be sent during this > > merge window once the sysctl and net trees land. Thomas Wei?schuh has > > it at the ready. :) > > Big fan of setting expectations :). thx for the comment. > Do you (@kees/ @thomas) have any preference on how to send the treewide > const patch? I have prepared wording for the pull request for when the > time comes next week, but if any of you prefer to send it through > another path different than sysctl, please let me know. I don't have any preference. I can only speak to historical context for other treewide changes: Linus has taken a PR, a raw patch, or just run a script directly in the past, so any should work. I would guess that a PR created from a script (that is reproduced in the commit log) is the easiest, as Linus can either just take the PR or choose to run the script himself. -- Kees Cook ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-16 14:16 ` [GIT PULL] sysctl changes for v6.11-rc1 Joel Granados 2024-07-16 18:13 ` Kees Cook @ 2024-07-16 21:43 ` pr-tracker-bot 2024-08-06 18:57 ` Solar Designer 2 siblings, 0 replies; 10+ messages in thread From: pr-tracker-bot @ 2024-07-16 21:43 UTC (permalink / raw) To: Joel Granados Cc: Linus Torvalds, Luis Chamberlain, Jeff Johnson, Joel Granados, Kees Cook, Thomas Weißschuh, Wen Yang, linux-kernel, linux-fsdevel The pull request you sent on Tue, 16 Jul 2024 16:16:56 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/ tags/sysctl-6.11-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/f8a8b94d0698ccc56c44478169c91ca774540d9f Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-07-16 14:16 ` [GIT PULL] sysctl changes for v6.11-rc1 Joel Granados 2024-07-16 18:13 ` Kees Cook 2024-07-16 21:43 ` pr-tracker-bot @ 2024-08-06 18:57 ` Solar Designer 2024-08-06 19:02 ` Linus Torvalds ` (2 more replies) 2 siblings, 3 replies; 10+ messages in thread From: Solar Designer @ 2024-08-06 18:57 UTC (permalink / raw) To: Joel Granados Cc: Linus Torvalds, Luis Chamberlain, Jeff Johnson, Kees Cook, Thomas Wei??schuh, Wen Yang, linux-kernel, linux-fsdevel On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > sysctl changes for 6.11-rc1 > > Summary > > * Remove "->procname == NULL" check when iterating through sysctl table arrays > > Removing sentinels in ctl_table arrays reduces the build time size and > runtime memory consumed by ~64 bytes per array. With all ctl_table > sentinels gone, the additional check for ->procname == NULL that worked in > tandem with the ARRAY_SIZE to calculate the size of the ctl_table arrays is > no longer needed and has been removed. The sysctl register functions now > returns an error if a sentinel is used. > > * Preparation patches for sysctl constification > > Constifying ctl_table structs prevents the modification of proc_handler > function pointers as they would reside in .rodata. The ctl_table arguments > in sysctl utility functions are const qualified in preparation for a future > treewide proc_handler argument constification commit. As (I assume it was) expected, these changes broke out-of-tree modules. For LKRG, I am repairing this by adding "#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,11,0)" checks around the corresponding module changes. This works. However, I wonder if it would possibly be better for the kernel to introduce a corresponding "feature test macro" (or two, for the two changes above). I worry that these changes (or some of them) could get backported to stable/longterm, which with the 6.11+ checks would unnecessarily break out-of-tree modules again (and again and again for each backport to a different kernel branch). Feature test macro(s) would avoid such further breakage, as they would (be supposed to be) included along with the backports. Joel, Linus, or anyone else - what do you think? And in general, would it be a good practice for Linux to be providing feature test macros to indicate this sort of changes? Is there a naming convention for them? For omitting the ctl_table array sentinel elements, it is now possible to check whether register_sysctl() is a function or a macro. I've tested the below and it works: +++ b/src/modules/comm_channel/p_comm_channel.c @@ -332,7 +332,14 @@ struct ctl_table p_lkrg_sysctl_table[] = { .extra1 = &p_profile_enforce_min, .extra2 = &p_profile_enforce_max, }, +/* + * Empty element at the end of array was required when register_sysctl() was a + * function. It's no longer required when it became a macro in 2023, and it's + * disallowed after further changes in 2024. + */ +#ifndef register_sysctl { } +#endif }; But it's a hack, which I'm unhappy about. So instead of a macro indicating that the "Remove "->procname == NULL" check when iterating through sysctl table arrays" change is in place, we could have one that indicates that the sentinel elements are no longer required (and no need for one indicating that they're no longer allowed, then). Something like LINUX_SYSCTL_NO_SENTINELS. This could even be backported to kernels that do not have the "Remove "->procname == NULL" check" commit, if they do have last year's removal of the requirement. Alternatively, maybe "Remove "->procname == NULL" check when iterating through sysctl table arrays" should be reverted. I can see how it's useful as a policy check for the kernel itself, so no space is inadvertently wasted on a sentinel element anywhere in the kernel tree, but maybe it isn't worth enforcing this for out-of-tree modules. The impact of an extra element (if allowed) is negligible, whereas the impact of not having it on an older kernel is really bad. I worry that some out-of-tree modules would be adapted or written for the new convention without a 6.11+ check, yet someone would also build and use them on pre-6.11. There's no compile-time failure from omitting the sentinel element on a kernel where it was needed, and there isn't a _reliable_ runtime failure either. The other macro could be called LINUX_SYSCTL_TABLE_CONST, although I'm not sure whether it should apply only to the "ctl_table arguments in sysctl utility functions" (the change so far) or also to "Constifying ctl_table structs" (a near future change, right?) Alexander ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-08-06 18:57 ` Solar Designer @ 2024-08-06 19:02 ` Linus Torvalds 2024-08-06 19:24 ` Thomas Weißschuh 2024-08-13 20:52 ` Joel Granados 2 siblings, 0 replies; 10+ messages in thread From: Linus Torvalds @ 2024-08-06 19:02 UTC (permalink / raw) To: Solar Designer Cc: Joel Granados, Luis Chamberlain, Jeff Johnson, Kees Cook, Thomas Wei??schuh, Wen Yang, linux-kernel, linux-fsdevel On Tue, 6 Aug 2024 at 11:57, Solar Designer <solar@openwall.com> wrote: > > As (I assume it was) expected, these changes broke out-of-tree modules. It was presumably not expected - but for the simple reason that no kernel developer should spend one single second worrying about out-of-tree modules. It's simply not a concern - never has been, and never will be. Now, if some out-of-tree module is on the cusp of being integrated, and is out-of-tree just because it's not quite ready yet, that would maybe be then a case of "hey, wait a second". But no. We are not going to start any kind of feature test macros for external modules in general. Linus ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-08-06 18:57 ` Solar Designer 2024-08-06 19:02 ` Linus Torvalds @ 2024-08-06 19:24 ` Thomas Weißschuh 2024-08-13 20:52 ` Joel Granados 2 siblings, 0 replies; 10+ messages in thread From: Thomas Weißschuh @ 2024-08-06 19:24 UTC (permalink / raw) To: Solar Designer Cc: Joel Granados, Linus Torvalds, Luis Chamberlain, Jeff Johnson, Kees Cook, Wen Yang, linux-kernel, linux-fsdevel On 2024-08-06 20:57:37+0000, Solar Designer wrote: > On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > > sysctl changes for 6.11-rc1 > > > > Summary > > > > * Remove "->procname == NULL" check when iterating through sysctl table arrays > > > > Removing sentinels in ctl_table arrays reduces the build time size and > > runtime memory consumed by ~64 bytes per array. With all ctl_table > > sentinels gone, the additional check for ->procname == NULL that worked in > > tandem with the ARRAY_SIZE to calculate the size of the ctl_table arrays is > > no longer needed and has been removed. The sysctl register functions now > > returns an error if a sentinel is used. > > > > * Preparation patches for sysctl constification > > > > Constifying ctl_table structs prevents the modification of proc_handler > > function pointers as they would reside in .rodata. The ctl_table arguments > > in sysctl utility functions are const qualified in preparation for a future > > treewide proc_handler argument constification commit. > > As (I assume it was) expected, these changes broke out-of-tree modules. > For LKRG, I am repairing this by adding "#if LINUX_VERSION_CODE >= > KERNEL_VERSION(6,11,0)" checks around the corresponding module changes. > This works. However, I wonder if it would possibly be better for the > kernel to introduce a corresponding "feature test macro" (or two, for > the two changes above). I worry that these changes (or some of them) > could get backported to stable/longterm, which with the 6.11+ checks > would unnecessarily break out-of-tree modules again (and again and again > for each backport to a different kernel branch). Feature test macro(s) > would avoid such further breakage, as they would (be supposed to be) > included along with the backports. I don't see any of these changes being backported. The removal of the "->procname == NULL" check depends on all in-kernel tables being registered with an explicit size, which is not the case on old kernels. So a backport would not only silently fail for external modules but also for internal code. The same for the constification patches but with build errors instead of runtime errors. My future sysctl constification patches will be backwards compatible at both compiletime and runtime, for both internal and external code. So the version checks should be enough here. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] sysctl changes for v6.11-rc1 2024-08-06 18:57 ` Solar Designer 2024-08-06 19:02 ` Linus Torvalds 2024-08-06 19:24 ` Thomas Weißschuh @ 2024-08-13 20:52 ` Joel Granados 2 siblings, 0 replies; 10+ messages in thread From: Joel Granados @ 2024-08-13 20:52 UTC (permalink / raw) To: Solar Designer Cc: Linus Torvalds, Luis Chamberlain, Jeff Johnson, Kees Cook, Thomas Wei??schuh, Wen Yang, linux-kernel, linux-fsdevel On Tue, Aug 06, 2024 at 08:57:37PM +0200, Solar Designer wrote: > On Tue, Jul 16, 2024 at 04:16:56PM +0200, Joel Granados wrote: > > sysctl changes for 6.11-rc1 > > > > Summary > > > > * Remove "->procname == NULL" check when iterating through sysctl table arrays > > > > Removing sentinels in ctl_table arrays reduces the build time size and > > runtime memory consumed by ~64 bytes per array. With all ctl_table > > sentinels gone, the additional check for ->procname == NULL that worked in > > tandem with the ARRAY_SIZE to calculate the size of the ctl_table arrays is > > no longer needed and has been removed. The sysctl register functions now > > returns an error if a sentinel is used. > > > > * Preparation patches for sysctl constification > > > > Constifying ctl_table structs prevents the modification of proc_handler > > function pointers as they would reside in .rodata. The ctl_table arguments > > in sysctl utility functions are const qualified in preparation for a future > > treewide proc_handler argument constification commit. > > As (I assume it was) expected, these changes broke out-of-tree modules. > For LKRG, I am repairing this by adding "#if LINUX_VERSION_CODE >= > KERNEL_VERSION(6,11,0)" checks around the corresponding module changes. > This works. However, I wonder if it would possibly be better for the > kernel to introduce a corresponding "feature test macro" (or two, for > the two changes above). I worry that these changes (or some of them) > could get backported to stable/longterm, which with the 6.11+ checks > would unnecessarily break out-of-tree modules again (and again and again > for each backport to a different kernel branch). Feature test macro(s) > would avoid such further breakage, as they would (be supposed to be) > included along with the backports. > > Joel, Linus, or anyone else - what do you think? And in general, would As mentioned by Thomas; These changed must not be backported and therefore there is not concern about backport consequences. > it be a good practice for Linux to be providing feature test macros to > indicate this sort of changes? Is there a naming convention for them? I don't think that would be a good practice. IMO, a good way to handle these things in out-of-tree modules is the LINUX_VERSION_CODE hack. You can see it here for the same reason : https://github.com/cryptodev-linux/cryptodev-linux/commit/99ae2a39ddc3f89c66d9f09783b591c0f2dbf2e9 ... Best -- Joel Granados ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-08-13 21:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20240716141703eucas1p2f6ddaf91b7363dd893d37b9aa8987dc6@eucas1p2.samsung.com>
2024-07-16 14:16 ` [GIT PULL] sysctl changes for v6.11-rc1 Joel Granados
2024-07-16 18:13 ` Kees Cook
2024-07-17 19:46 ` Joel Granados
2024-07-17 20:05 ` Thomas Weißschuh
2024-07-17 22:15 ` Kees Cook
2024-07-16 21:43 ` pr-tracker-bot
2024-08-06 18:57 ` Solar Designer
2024-08-06 19:02 ` Linus Torvalds
2024-08-06 19:24 ` Thomas Weißschuh
2024-08-13 20:52 ` Joel Granados
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).