Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
On Tue, Apr 23, 2024 at 09:54:35AM +0200, Thomas Weißschuh wrote: > * Patch 1 is a bugfix for the stack_erasing sysctl handler > * Patches 2-10 change various helper functions throughout the kernel to > be able to handle 'const ctl_table'. > * Patch 11 changes the signatures of all proc handlers through the tree. > Some other signatures are also adapted, for details see the commit > message. > > Only patch 1 changes any code at all. > > The series was compile-tested on top of next-20230423 for > i386, x86_64, arm, arm64, riscv, loongarch, s390 and m68k. > > The series was split from my larger series sysctl-const series [0]. > It only focusses on the proc_handlers but is an important step to be > able to move all static definitions of ctl_table into .rodata. > > [0] > https://lore.kernel.org/lkml/20231204-const-sysctl-v2-0-7a5060b11...@weissschuh.net/ > > Signed-off-by: Thomas Weißschuh Cover letters don't need SOBS we only use them for patches. But anyway: Reviewed-by: Luis Chamberlain Luis
Re: [apparmor] [PATCH 01/79] fs: add ctime accessors infrastructure
On Wed, Jun 21, 2023 at 10:45:06AM -0400, Jeff Layton wrote: > struct timespec64 has unused bits in the tv_nsec field that can be used > for other purposes. In future patches, we're going to change how the > inode->i_ctime is accessed in certain inodes in order to make use of > them. In order to do that safely though, we'll need to eradicate raw > accesses of the inode->i_ctime field from the kernel. > > Add new accessor functions for the ctime that we can use to replace them. > > Signed-off-by: Jeff Layton Reviewed-by: Luis Chamberlain Luis
Re: [apparmor] [PATCH 00/79] fs: new accessors for inode->i_ctime
On Wed, Jun 21, 2023 at 03:21:41PM -0400, Steven Rostedt wrote: > On Wed, 21 Jun 2023 10:45:05 -0400 > Jeff Layton wrote: > > > Most of this conversion was done via coccinelle, with a few of the more > > non-standard accesses done by hand. There should be no behavioral > > changes with this set. That will come later, as we convert individual > > filesystems to use multigrain timestamps. > > BTW, Linus has suggested to me that whenever a conccinelle script is used, > it should be included in the change log. Sometimes people like the coccinelle included in the commit, sometimes people don't [0], it really ends up being up to a subjective maintainer preference. A compromise could be to use git notes as these are optional, however if we want to go down that path we should try to make a general consensus on it so we can send a consistent message. [0] https://lore.kernel.org/all/20230512073100.gc32...@twin.jikos.cz/ Luis
Re: [apparmor] [PATCH 08/11] kernel: pid_namespace: simplify sysctls with register_sysctl()
On Thu, Mar 09, 2023 at 02:27:46PM -0800, Andrew Morton wrote: > On Thu, 9 Mar 2023 14:11:27 -0800 Luis Chamberlain wrote: > > > Andrew, kernel/pid_sysctl.h is new, not on v6.3-rc1 and so I cannot > > carry this on sysctl-next. Can you carry this patch on your tree? > > Sure, no probs. Andrew, this one patch will have to go through your tree as kernel/pid_sysctl.h only exist on your tree. I'll drop it on my end! Thanks! Luis
Re: [apparmor] [PATCH 08/11] kernel: pid_namespace: simplify sysctls with register_sysctl()
On Thu, Mar 09, 2023 at 08:14:33PM -0800, Eric Biggers wrote: > On Thu, Mar 09, 2023 at 02:11:27PM -0800, Luis Chamberlain wrote: > > On Thu, Mar 02, 2023 at 03:13:54PM -0800, Jeff Xu wrote: > > > On Thu, Mar 2, 2023 at 12:28 PM Luis Chamberlain > > > wrote: > > > > kernel/pid_sysctl.h| 3 +-- > > > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > > > > Acked-by: Jeff Xu > > > > Andrew, kernel/pid_sysctl.h is new, not on v6.3-rc1 and so I cannot > > carry this on sysctl-next. Can you carry this patch on your tree? > > > > I see Eric Biggers already took in the fs-verity patch, so I will drop > > that from my queue. > > > > I can take the rest in this series. > > > > I will also hold off on the last patch which deprecates the routine > > register_sysctl_paths() until after say the first part of the merge > > window. > > > > This will allow all of our trees to work on linux-next without conflict. > > > > Let me know if this is OK with you and Eric! > > > > That's fine with me. I applied the fsverity patch based on your cover letter > that said it was okay > (https://lore.kernel.org/r/20230302202826.776286-1-mcg...@kernel.org). Yeah it perfectly fine! > If you'd > like to take all the patches so that you can remove register_sysctl_paths() in > the same cycle, that would be fine too; it's up to you. Nah it's fine, no rush in this. One small step at a time. Luis
Re: [apparmor] [PATCH 11/11] proc_sysctl: deprecate register_sysctl_paths()
On Thu, Mar 09, 2023 at 08:17:07PM -0800, Eric Biggers wrote:
> On Thu, Mar 02, 2023 at 12:28:26PM -0800, Luis Chamberlain wrote:
> > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > index 780690dc08cd..e8459fc56b50 100644
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -221,13 +221,8 @@ extern void retire_sysctl_set(struct ctl_table_set
> > *set);
> > struct ctl_table_header *__register_sysctl_table(
> > struct ctl_table_set *set,
> > const char *path, struct ctl_table *table);
> > -struct ctl_table_header *__register_sysctl_paths(
> > - struct ctl_table_set *set,
> > - const struct ctl_path *path, struct ctl_table *table);
> > struct ctl_table_header *register_sysctl(const char *path, struct
> > ctl_table *table);
> > struct ctl_table_header *register_sysctl_table(struct ctl_table * table);
> > -struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
> > - struct ctl_table *table);
> >
> > void unregister_sysctl_table(struct ctl_table_header * table);
> >
> > @@ -277,12 +272,6 @@ static inline struct ctl_table_header
> > *register_sysctl_mount_point(const char *p
> > return NULL;
> > }
> >
> > -static inline struct ctl_table_header *register_sysctl_paths(
> > - const struct ctl_path *path, struct ctl_table *table)
> > -{
> > - return NULL;
> > -}
> > -
>
> Seems that this patch should be titled "remove register_sysctl_paths()", not
> "deprecate register_sysctl_paths()"?
Good call! Will adjust.
Luis
Re: [apparmor] [PATCH 0/7] sysctl: slowly deprecate register_sysctl_table()
On Thu, Mar 02, 2023 at 12:46:05PM -0800, Luis Chamberlain wrote: > I'm happy to take these via sysctl-next [0] but since > I don' think register_sysctl_table() will be nuked on v6.4 I think > it's fine for each of these to go into each respective tree. I can > pick up last stragglers on sysctl-next. If you want me to take this > via sysctl-next too, just let me know and I'm happy to do that. Either > way works. As I noted I've dropped the following already-picked-up patches from my queue: ipmi: simplify sysctl registration sgi-xp: simplify sysctl registration tty: simplify sysctl registration I've taken the rest now through sysctl-next: scsi: simplify sysctl registration with register_sysctl() hv: simplify sysctl registration md: simplify sysctl registration xen: simplify sysctl registration for balloon If a maintainer would prefer to take one on through their tree fine by me too, just let me know and I'll drop the patch. Luis
Re: [apparmor] [PATCH 08/11] kernel: pid_namespace: simplify sysctls with register_sysctl()
On Thu, Mar 02, 2023 at 03:13:54PM -0800, Jeff Xu wrote: > On Thu, Mar 2, 2023 at 12:28 PM Luis Chamberlain wrote: > > kernel/pid_sysctl.h| 3 +-- > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > Acked-by: Jeff Xu Andrew, kernel/pid_sysctl.h is new, not on v6.3-rc1 and so I cannot carry this on sysctl-next. Can you carry this patch on your tree? I see Eric Biggers already took in the fs-verity patch, so I will drop that from my queue. I can take the rest in this series. I will also hold off on the last patch which deprecates the routine register_sysctl_paths() until after say the first part of the merge window. This will allow all of our trees to work on linux-next without conflict. Let me know if this is OK with you and Eric! Luis
[apparmor] [PATCH 2/7] ipmi: simplify sysctl registration
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl() can do the directory creation for you so just use
that.
Signed-off-by: Luis Chamberlain
---
drivers/char/ipmi/ipmi_poweroff.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_poweroff.c
b/drivers/char/ipmi/ipmi_poweroff.c
index 163ec9749e55..870659d91db2 100644
--- a/drivers/char/ipmi/ipmi_poweroff.c
+++ b/drivers/char/ipmi/ipmi_poweroff.c
@@ -659,20 +659,6 @@ static struct ctl_table ipmi_table[] = {
{ }
};
-static struct ctl_table ipmi_dir_table[] = {
- { .procname = "ipmi",
- .mode = 0555,
- .child= ipmi_table },
- { }
-};
-
-static struct ctl_table ipmi_root_table[] = {
- { .procname = "dev",
- .mode = 0555,
- .child= ipmi_dir_table },
- { }
-};
-
static struct ctl_table_header *ipmi_table_header;
#endif /* CONFIG_PROC_FS */
@@ -689,7 +675,7 @@ static int __init ipmi_poweroff_init(void)
pr_info("Power cycle is enabled\n");
#ifdef CONFIG_PROC_FS
- ipmi_table_header = register_sysctl_table(ipmi_root_table);
+ ipmi_table_header = register_sysctl("dev/ipmi", ipmi_table);
if (!ipmi_table_header) {
pr_err("Unable to register powercycle sysctl\n");
rv = -ENOMEM;
--
2.39.1
[apparmor] [PATCH 0/7] sysctl: slowly deprecate register_sysctl_table()
As the large array of sysctls in kernel/sysctl.c is reduced we get to the point of wanting to optimize how we register sysctls by only dealing with flat simple structures, with no subdirectories. In particular the last empty element should not be needed. We'll get there, and save some memory, but as we move forward that path will be come the more relevant path to use in the sysctl registration. It is much simpler as it avoids recursion. Turns out we can also convert existing users of register_sysctl_table() which just need their subdirectories created for them. This effort addresses most users of register_sysctl_table() in drivers/ except parport -- that needs a bit more review. This is part of the process to deprecate older sysctl users which uses APIs which can incur recursion, but don't need it [0]. This is the second effort. Yes -- we'll get to the point *each* of these conversions means saving one empty syctl, but that change needs a bit more careful review before merging. But since these conversion are also deleting tables for subdirectories, the delta in size of the kernel should not incrase really. The most complex change is the sgi-xp change which does deal with a case where we have a subdirectory with an entry, I just split that in two registrations. No point in keeping recursion just for a few minor if we can simplify code around. More eyeballs / review / testing on that change is appreciated. Sending these out early so they can get tested properly early on linux-next. I'm happy to take these via sysctl-next [0] but since I don' think register_sysctl_table() will be nuked on v6.4 I think it's fine for each of these to go into each respective tree. I can pick up last stragglers on sysctl-next. If you want me to take this via sysctl-next too, just let me know and I'm happy to do that. Either way works. [0] https://lkml.kernel.org/r/20230302202826.776286-1-mcg...@kernel.org Luis Chamberlain (7): scsi: simplify sysctl registration with register_sysctl() ipmi: simplify sysctl registration hv: simplify sysctl registration md: simplify sysctl registration sgi-xp: simplify sysctl registration tty: simplify sysctl registration xen: simplify sysctl registration for balloon drivers/char/ipmi/ipmi_poweroff.c | 16 +--- drivers/hv/vmbus_drv.c| 11 +-- drivers/md/md.c | 22 +- drivers/misc/sgi-xp/xpc_main.c| 24 ++-- drivers/scsi/scsi_sysctl.c| 16 +--- drivers/tty/tty_io.c | 20 +--- drivers/xen/balloon.c | 20 +--- 7 files changed, 16 insertions(+), 113 deletions(-) -- 2.39.1
[apparmor] [PATCH 5/7] sgi-xp: simplify sysctl registration
Although this driver is a good use case for having a directory
that is not other directories and then subdirectories with more
entries, the usage of register_sysctl_table() can recurse and
increases complexity so to avoid that just split out the
registration to each directory with its own entries.
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl() can do the directory creation for you so just use
that.
Signed-off-by: Luis Chamberlain
---
drivers/misc/sgi-xp/xpc_main.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/sgi-xp/xpc_main.c b/drivers/misc/sgi-xp/xpc_main.c
index b2c3c22fc13c..6da509d692bb 100644
--- a/drivers/misc/sgi-xp/xpc_main.c
+++ b/drivers/misc/sgi-xp/xpc_main.c
@@ -93,7 +93,7 @@ int xpc_disengage_timelimit = XPC_DISENGAGE_DEFAULT_TIMELIMIT;
static int xpc_disengage_min_timelimit;/* = 0 */
static int xpc_disengage_max_timelimit = 120;
-static struct ctl_table xpc_sys_xpc_hb_dir[] = {
+static struct ctl_table xpc_sys_xpc_hb[] = {
{
.procname = "hb_interval",
.data = &xpc_hb_interval,
@@ -112,11 +112,7 @@ static struct ctl_table xpc_sys_xpc_hb_dir[] = {
.extra2 = &xpc_hb_check_max_interval},
{}
};
-static struct ctl_table xpc_sys_xpc_dir[] = {
- {
-.procname = "hb",
-.mode = 0555,
-.child = xpc_sys_xpc_hb_dir},
+static struct ctl_table xpc_sys_xpc[] = {
{
.procname = "disengage_timelimit",
.data = &xpc_disengage_timelimit,
@@ -127,14 +123,9 @@ static struct ctl_table xpc_sys_xpc_dir[] = {
.extra2 = &xpc_disengage_max_timelimit},
{}
};
-static struct ctl_table xpc_sys_dir[] = {
- {
-.procname = "xpc",
-.mode = 0555,
-.child = xpc_sys_xpc_dir},
- {}
-};
+
static struct ctl_table_header *xpc_sysctl;
+static struct ctl_table_header *xpc_sysctl_hb;
/* non-zero if any remote partition disengage was timed out */
int xpc_disengage_timedout;
@@ -1041,6 +1032,8 @@ xpc_do_exit(enum xp_retval reason)
if (xpc_sysctl)
unregister_sysctl_table(xpc_sysctl);
+ if (xpc_sysctl_hb)
+ unregister_sysctl_table(xpc_sysctl_hb);
xpc_teardown_partitions();
@@ -1243,7 +1236,8 @@ xpc_init(void)
goto out_1;
}
- xpc_sysctl = register_sysctl_table(xpc_sys_dir);
+ xpc_sysctl = register_sysctl("xpc", xpc_sys_xpc);
+ xpc_sysctl_hb = register_sysctl("xpc/hb", xpc_sys_xpc_hb);
/*
* Fill the partition reserved page with the information needed by
@@ -1308,6 +1302,8 @@ xpc_init(void)
(void)unregister_die_notifier(&xpc_die_notifier);
(void)unregister_reboot_notifier(&xpc_reboot_notifier);
out_2:
+ if (xpc_sysctl_hb)
+ unregister_sysctl_table(xpc_sysctl_hb);
if (xpc_sysctl)
unregister_sysctl_table(xpc_sysctl);
--
2.39.1
[apparmor] [PATCH 03/11] sysctl: clarify register_sysctl_init() base directory order
Relatively new docs which I added which hinted the base directories needed to be created before is wrong, remove that incorrect comment. This has been hinted before by Eric twice already [0] [1], I had just not verified that until now. Now that I've verified that updates the docs to relax the context described. [0] https://lkml.kernel.org/r/875ys0azt8@email.froward.int.ebiederm.org [1] https://lkml.kernel.org/r/87ftbiud6s@x220.int.ebiederm.org Suggested-by: Eric W. Biederman Signed-off-by: Luis Chamberlain --- fs/proc/proc_sysctl.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 6b9b2694d430..15d5e02f1ec0 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1440,10 +1440,7 @@ EXPORT_SYMBOL(register_sysctl); * register_sysctl() failing on init are extremely low, and so for both reasons * this function does not return any error as it is used by initialization code. * - * Context: Can only be called after your respective sysctl base path has been - * registered. So for instance, most base directories are registered early on - * init before init levels are processed through proc_sys_init() and - * sysctl_init_bases(). + * Context: if your base directory does not exist it will be created for you. */ void __init __register_sysctl_init(const char *path, struct ctl_table *table, const char *table_name) -- 2.39.1
[apparmor] [PATCH 08/11] kernel: pid_namespace: simplify sysctls with register_sysctl()
register_sysctl_paths() is only required if your child (directories)
have entries and pid_namespace does not. So use register_sysctl_init()
instead where we don't care about the return value and use
register_sysctl() where we do.
Signed-off-by: Luis Chamberlain
---
kernel/pid_namespace.c | 3 +--
kernel/pid_sysctl.h| 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 46e0d5a3f91f..b43eee07b00c 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -314,7 +314,6 @@ static struct ctl_table pid_ns_ctl_table[] = {
},
{ }
};
-static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
#endif /* CONFIG_CHECKPOINT_RESTORE */
int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
@@ -473,7 +472,7 @@ static __init int pid_namespaces_init(void)
pid_ns_cachep = KMEM_CACHE(pid_namespace, SLAB_PANIC | SLAB_ACCOUNT);
#ifdef CONFIG_CHECKPOINT_RESTORE
- register_sysctl_paths(kern_path, pid_ns_ctl_table);
+ register_sysctl_init("kernel", pid_ns_ctl_table);
#endif
register_pid_ns_sysctl_table_vm();
diff --git a/kernel/pid_sysctl.h b/kernel/pid_sysctl.h
index e22d072e1e24..d67a4d45bb42 100644
--- a/kernel/pid_sysctl.h
+++ b/kernel/pid_sysctl.h
@@ -46,10 +46,9 @@ static struct ctl_table pid_ns_ctl_table_vm[] = {
},
{ }
};
-static struct ctl_path vm_path[] = { { .procname = "vm", }, { } };
static inline void register_pid_ns_sysctl_table_vm(void)
{
- register_sysctl_paths(vm_path, pid_ns_ctl_table_vm);
+ register_sysctl("vm", pid_ns_ctl_table_vm);
}
#else
static inline void initialize_memfd_noexec_scope(struct pid_namespace *ns) {}
--
2.39.1
[apparmor] [PATCH 04/11] apparmor: simplify sysctls with register_sysctl_init()
Using register_sysctl_paths() is really only needed if you have
subdirectories with entries. We can use the simple register_sysctl()
instead.
Signed-off-by: Luis Chamberlain
---
security/apparmor/lsm.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index d6cc4812ca53..47c7ec7e5a80 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1764,11 +1764,6 @@ static int apparmor_dointvec(struct ctl_table *table,
int write,
return proc_dointvec(table, write, buffer, lenp, ppos);
}
-static struct ctl_path apparmor_sysctl_path[] = {
- { .procname = "kernel", },
- { }
-};
-
static struct ctl_table apparmor_sysctl_table[] = {
{
.procname = "unprivileged_userns_apparmor_policy",
@@ -1790,8 +1785,7 @@ static struct ctl_table apparmor_sysctl_table[] = {
static int __init apparmor_init_sysctl(void)
{
- return register_sysctl_paths(apparmor_sysctl_path,
-apparmor_sysctl_table) ? 0 : -ENOMEM;
+ return register_sysctl("kernel", apparmor_sysctl_table) ? 0 : -ENOMEM;
}
#else
static inline int apparmor_init_sysctl(void)
--
2.39.1
[apparmor] [PATCH 02/11] proc_sysctl: move helper which creates required subdirectories
Move the code which creates the subdirectories for a ctl table
into a helper routine so to make it easier to review. Document
the goal.
This creates no functional changes.
Signed-off-by: Luis Chamberlain
---
fs/proc/proc_sysctl.c | 56 ---
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 1df0beb50dbe..6b9b2694d430 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1283,6 +1283,35 @@ static int insert_links(struct ctl_table_header *head)
return err;
}
+/* Find the directory for the ctl_table. If one is not found create it. */
+static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
+{
+ const char *name, *nextname;
+
+ for (name = path; name; name = nextname) {
+ int namelen;
+ nextname = strchr(name, '/');
+ if (nextname) {
+ namelen = nextname - name;
+ nextname++;
+ } else {
+ namelen = strlen(name);
+ }
+ if (namelen == 0)
+ continue;
+
+ /*
+* namelen ensures if name is "foo/bar/yay" only foo is
+* registered first. We traverse as if using mkdir -p and
+* return a ctl_dir for the last directory entry.
+*/
+ dir = get_subdir(dir, name, namelen);
+ if (IS_ERR(dir))
+ break;
+ }
+ return dir;
+}
+
/**
* __register_sysctl_table - register a leaf sysctl table
* @set: Sysctl tree to register on
@@ -1334,7 +1363,6 @@ struct ctl_table_header *__register_sysctl_table(
{
struct ctl_table_root *root = set->dir.header.root;
struct ctl_table_header *header;
- const char *name, *nextname;
struct ctl_dir *dir;
struct ctl_table *entry;
struct ctl_node *node;
@@ -1359,29 +1387,9 @@ struct ctl_table_header *__register_sysctl_table(
dir->header.nreg++;
spin_unlock(&sysctl_lock);
- /* Find the directory for the ctl_table */
- for (name = path; name; name = nextname) {
- int namelen;
- nextname = strchr(name, '/');
- if (nextname) {
- namelen = nextname - name;
- nextname++;
- } else {
- namelen = strlen(name);
- }
- if (namelen == 0)
- continue;
-
- /*
-* namelen ensures if name is "foo/bar/yay" only foo is
-* registered first. We traverse as if using mkdir -p and
-* return a ctl_dir for the last directory entry.
-*/
- dir = get_subdir(dir, name, namelen);
- if (IS_ERR(dir))
- goto fail;
- }
-
+ dir = sysctl_mkdir_p(dir, path);
+ if (IS_ERR(dir))
+ goto fail;
spin_lock(&sysctl_lock);
if (insert_header(dir, header))
goto fail_put_dir_locked;
--
2.39.1
[apparmor] [PATCH 05/11] loadpin: simplify sysctls use with register_sysctl()
register_sysctl_paths() is not required, we can just use
register_sysctl() with the required path specified.
Signed-off-by: Luis Chamberlain
---
security/loadpin/loadpin.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index d73a281adf86..c971464b4ad5 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -52,12 +52,6 @@ static bool deny_reading_verity_digests;
#endif
#ifdef CONFIG_SYSCTL
-static struct ctl_path loadpin_sysctl_path[] = {
- { .procname = "kernel", },
- { .procname = "loadpin", },
- { }
-};
-
static struct ctl_table loadpin_sysctl_table[] = {
{
.procname = "enforce",
@@ -262,7 +256,7 @@ static int __init loadpin_init(void)
enforce ? "" : "not ");
parse_exclude();
#ifdef CONFIG_SYSCTL
- if (!register_sysctl_paths(loadpin_sysctl_path, loadpin_sysctl_table))
+ if (!register_sysctl("kernel/loadpin", loadpin_sysctl_table))
pr_notice("sysctl registration failed!\n");
#endif
security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
--
2.39.1
[apparmor] [PATCH 09/11] fs-verity: simplify sysctls with register_sysctl()
register_sysctl_paths() is only needed if your child (directories) have
entries but this does not so just use register_sysctl() so to do away
with the path specification.
Signed-off-by: Luis Chamberlain
---
fs/verity/signature.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index e7d3ca919a1e..b8c51ad40d3a 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -88,12 +88,6 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
#ifdef CONFIG_SYSCTL
static struct ctl_table_header *fsverity_sysctl_header;
-static const struct ctl_path fsverity_sysctl_path[] = {
- { .procname = "fs", },
- { .procname = "verity", },
- { }
-};
-
static struct ctl_table fsverity_sysctl_table[] = {
{
.procname = "require_signatures",
@@ -109,8 +103,7 @@ static struct ctl_table fsverity_sysctl_table[] = {
static int __init fsverity_sysctl_init(void)
{
- fsverity_sysctl_header = register_sysctl_paths(fsverity_sysctl_path,
- fsverity_sysctl_table);
+ fsverity_sysctl_header = register_sysctl("fs/verity",
fsverity_sysctl_table);
if (!fsverity_sysctl_header) {
pr_err("sysctl registration failed!\n");
return -ENOMEM;
--
2.39.1
[apparmor] [PATCH 06/11] yama: simplfy sysctls with register_sysctl()
register_sysctl_paths() is only need if you have directories with
entries, simplify this by using register_sysctl().
Signed-off-by: Luis Chamberlain
---
security/yama/yama_lsm.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 06e226166aab..90dd012b0db5 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -447,12 +447,6 @@ static int yama_dointvec_minmax(struct ctl_table *table,
int write,
static int max_scope = YAMA_SCOPE_NO_ATTACH;
-static struct ctl_path yama_sysctl_path[] = {
- { .procname = "kernel", },
- { .procname = "yama", },
- { }
-};
-
static struct ctl_table yama_sysctl_table[] = {
{
.procname = "ptrace_scope",
@@ -467,7 +461,7 @@ static struct ctl_table yama_sysctl_table[] = {
};
static void __init yama_init_sysctl(void)
{
- if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table))
+ if (!register_sysctl("kernel/yama", yama_sysctl_table))
panic("Yama: sysctl registration failed.\n");
}
#else
--
2.39.1
[apparmor] [PATCH 10/11] csky: simplify alignment sysctl registration
Using register_sysctl_paths() is only required if we are using
leafs with entries but all we are doing is creates leafs with
just one leaf and then entries and register_sysctl_init() works
well with that already.
The 555 permission is already retained by the new_dir() proc
sysctl directory creator.
Signed-off-by: Luis Chamberlain
---
arch/csky/abiv1/alignment.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/arch/csky/abiv1/alignment.c b/arch/csky/abiv1/alignment.c
index 2df115d0e210..b60259daed1b 100644
--- a/arch/csky/abiv1/alignment.c
+++ b/arch/csky/abiv1/alignment.c
@@ -332,22 +332,9 @@ static struct ctl_table alignment_tbl[5] = {
{}
};
-static struct ctl_table sysctl_table[2] = {
- {
-.procname = "csky_alignment",
-.mode = 0555,
-.child = alignment_tbl},
- {}
-};
-
-static struct ctl_path sysctl_path[2] = {
- {.procname = "csky"},
- {}
-};
-
static int __init csky_alignment_init(void)
{
- register_sysctl_paths(sysctl_path, sysctl_table);
+ register_sysctl_init("csky/csky_alignment", alignment_tbl);
return 0;
}
--
2.39.1
[apparmor] [PATCH 11/11] proc_sysctl: deprecate register_sysctl_paths()
Now that all users are removed, drop the export for register_sysctl_paths()
Signed-off-by: Luis Chamberlain
---
fs/proc/proc_sysctl.c | 30 +-
include/linux/sysctl.h | 11 ---
2 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 15d5e02f1ec0..ff06434f7be1 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1486,6 +1486,7 @@ static int count_subheaders(struct ctl_table *table)
return nr_subheaders + has_files;
}
+/* Note: this can recurse and call itself when dealing with subdirectories */
static int register_leaf_sysctl_tables(const char *path, char *pos,
struct ctl_table_header ***subheader, struct ctl_table_set *set,
struct ctl_table *table)
@@ -1571,9 +1572,10 @@ static int register_leaf_sysctl_tables(const char *path,
char *pos,
* Register a sysctl table hierarchy. @table should be a filled in ctl_table
* array. A completely 0 filled entry terminates the table.
*
- * See __register_sysctl_table for more details.
+ * See __register_sysctl_table for more details. This routine can
+ * recurse by having register_leaf_sysctl_tables() call itself.
*/
-struct ctl_table_header *__register_sysctl_paths(
+static struct ctl_table_header *__register_sysctl_paths(
struct ctl_table_set *set,
const struct ctl_path *path, struct ctl_table *table)
{
@@ -1613,6 +1615,7 @@ struct ctl_table_header *__register_sysctl_paths(
subheader = subheaders;
header->ctl_table_arg = ctl_table_arg;
+ /* this can recurse */
if (register_leaf_sysctl_tables(new_path, pos, &subheader,
set, table))
goto err_register_leaves;
@@ -1635,37 +1638,22 @@ struct ctl_table_header *__register_sysctl_paths(
}
/**
- * register_sysctl_paths - register a sysctl table hierarchy
- * @path: The path to the directory the sysctl table is in.
+ * register_sysctl_table - register a sysctl table hierarchy
* @table: the top-level table structure
*
* Register a sysctl table hierarchy. @table should be a filled in ctl_table
* array. A completely 0 filled entry terminates the table.
*
* See __register_sysctl_paths for more details.
- */
-struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
- struct ctl_table *table)
-{
- return __register_sysctl_paths(&sysctl_table_root.default_set,
- path, table);
-}
-EXPORT_SYMBOL(register_sysctl_paths);
-
-/**
- * register_sysctl_table - register a sysctl table hierarchy
- * @table: the top-level table structure
- *
- * Register a sysctl table hierarchy. @table should be a filled in ctl_table
- * array. A completely 0 filled entry terminates the table.
*
- * See register_sysctl_paths for more details.
+ * This is a deprecated compatibility wrapper. You should avoid adding new
+ * users of this into the kernel.
*/
struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
{
static const struct ctl_path null_path[] = { {} };
- return register_sysctl_paths(null_path, table);
+ return __register_sysctl_paths(&sysctl_table_root.default_set,
null_path, table);
}
EXPORT_SYMBOL(register_sysctl_table);
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 780690dc08cd..e8459fc56b50 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -221,13 +221,8 @@ extern void retire_sysctl_set(struct ctl_table_set *set);
struct ctl_table_header *__register_sysctl_table(
struct ctl_table_set *set,
const char *path, struct ctl_table *table);
-struct ctl_table_header *__register_sysctl_paths(
- struct ctl_table_set *set,
- const struct ctl_path *path, struct ctl_table *table);
struct ctl_table_header *register_sysctl(const char *path, struct ctl_table
*table);
struct ctl_table_header *register_sysctl_table(struct ctl_table * table);
-struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
- struct ctl_table *table);
void unregister_sysctl_table(struct ctl_table_header * table);
@@ -277,12 +272,6 @@ static inline struct ctl_table_header
*register_sysctl_mount_point(const char *p
return NULL;
}
-static inline struct ctl_table_header *register_sysctl_paths(
- const struct ctl_path *path, struct ctl_table *table)
-{
- return NULL;
-}
-
static inline struct ctl_table_header *register_sysctl(const char *path,
struct ctl_table *table)
{
return NULL;
--
2.39.1
[apparmor] [PATCH 1/7] scsi: simplify sysctl registration with register_sysctl()
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl() can do the directory creation for you so just use that.
Signed-off-by: Luis Chamberlain
---
drivers/scsi/scsi_sysctl.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/scsi/scsi_sysctl.c b/drivers/scsi/scsi_sysctl.c
index 7259704a7f52..7f0914ea168f 100644
--- a/drivers/scsi/scsi_sysctl.c
+++ b/drivers/scsi/scsi_sysctl.c
@@ -21,25 +21,11 @@ static struct ctl_table scsi_table[] = {
{ }
};
-static struct ctl_table scsi_dir_table[] = {
- { .procname = "scsi",
- .mode = 0555,
- .child= scsi_table },
- { }
-};
-
-static struct ctl_table scsi_root_table[] = {
- { .procname = "dev",
- .mode = 0555,
- .child= scsi_dir_table },
- { }
-};
-
static struct ctl_table_header *scsi_table_header;
int __init scsi_init_sysctl(void)
{
- scsi_table_header = register_sysctl_table(scsi_root_table);
+ scsi_table_header = register_sysctl("dev/scsi", scsi_table);
if (!scsi_table_header)
return -ENOMEM;
return 0;
--
2.39.1
[apparmor] [PATCH 07/11] seccomp: simplify sysctls with register_sysctl_init()
register_sysctl_paths() is only needed if you have childs (directories)
with entries. Just use register_sysctl_init() as it also does the
kmemleak check for you.
Signed-off-by: Luis Chamberlain
---
kernel/seccomp.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index cebf26445f9e..d3e584065c7f 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -2368,12 +2368,6 @@ static int seccomp_actions_logged_handler(struct
ctl_table *ro_table, int write,
return ret;
}
-static struct ctl_path seccomp_sysctl_path[] = {
- { .procname = "kernel", },
- { .procname = "seccomp", },
- { }
-};
-
static struct ctl_table seccomp_sysctl_table[] = {
{
.procname = "actions_avail",
@@ -2392,14 +2386,7 @@ static struct ctl_table seccomp_sysctl_table[] = {
static int __init seccomp_sysctl_init(void)
{
- struct ctl_table_header *hdr;
-
- hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
- if (!hdr)
- pr_warn("sysctl registration failed\n");
- else
- kmemleak_not_leak(hdr);
-
+ register_sysctl_init("kernel/seccomp", seccomp_sysctl_table);
return 0;
}
--
2.39.1
[apparmor] [PATCH 4/7] md: simplify sysctl registration
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl() can do the directory creation for you so just use
that.
Signed-off-by: Luis Chamberlain
---
drivers/md/md.c | 22 +-
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 927a43db5dfb..546b1b81eb28 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -322,26 +322,6 @@ static struct ctl_table raid_table[] = {
{ }
};
-static struct ctl_table raid_dir_table[] = {
- {
- .procname = "raid",
- .maxlen = 0,
- .mode = S_IRUGO|S_IXUGO,
- .child = raid_table,
- },
- { }
-};
-
-static struct ctl_table raid_root_table[] = {
- {
- .procname = "dev",
- .maxlen = 0,
- .mode = 0555,
- .child = raid_dir_table,
- },
- { }
-};
-
static int start_readonly;
/*
@@ -9650,7 +9630,7 @@ static int __init md_init(void)
mdp_major = ret;
register_reboot_notifier(&md_notifier);
- raid_table_header = register_sysctl_table(raid_root_table);
+ raid_table_header = register_sysctl("dev/raid", raid_table);
md_geninit();
return 0;
--
2.39.1
[apparmor] [PATCH 3/7] hv: simplify sysctl registration
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl() can do the directory creation for you so just use
that.
Signed-off-by: Luis Chamberlain
---
drivers/hv/vmbus_drv.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index d24dd65b33d4..229353f1e9c2 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1460,15 +1460,6 @@ static struct ctl_table hv_ctl_table[] = {
{}
};
-static struct ctl_table hv_root_table[] = {
- {
- .procname = "kernel",
- .mode = 0555,
- .child = hv_ctl_table
- },
- {}
-};
-
/*
* vmbus_bus_init -Main vmbus driver initialization routine.
*
@@ -1547,7 +1538,7 @@ static int vmbus_bus_init(void)
* message recording won't be available in isolated
* guests should the following registration fail.
*/
- hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
+ hv_ctl_table_hdr = register_sysctl("kernel", hv_ctl_table);
if (!hv_ctl_table_hdr)
pr_err("Hyper-V: sysctl table register error");
--
2.39.1
[apparmor] [PATCH 7/7] xen: simplify sysctl registration for balloon
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl_init() can do the directory creation for you so just
use that.
Signed-off-by: Luis Chamberlain
---
drivers/xen/balloon.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 617a7f4f07a8..586a1673459e 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -97,24 +97,6 @@ static struct ctl_table balloon_table[] = {
{ }
};
-static struct ctl_table balloon_root[] = {
- {
- .procname = "balloon",
- .mode = 0555,
- .child = balloon_table,
- },
- { }
-};
-
-static struct ctl_table xen_root[] = {
- {
- .procname = "xen",
- .mode = 0555,
- .child = balloon_root,
- },
- { }
-};
-
#else
#define xen_hotplug_unpopulated 0
#endif
@@ -747,7 +729,7 @@ static int __init balloon_init(void)
#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
set_online_page_callback(&xen_online_page);
register_memory_notifier(&xen_memory_nb);
- register_sysctl_table(xen_root);
+ register_sysctl_init("xen/balloon", balloon_table);
#endif
balloon_add_regions();
--
2.39.1
[apparmor] [PATCH 01/11] proc_sysctl: update docs for __register_sysctl_table()
Update the docs for __register_sysctl_table() to make it clear no child entries can be passed. When the child is true these are non-leaf entries on the ctl table and sysctl treats these as directories. The point to __register_sysctl_table() is to deal only with directories not part of the ctl table where thay may riside, to be simple and avoid recursion. While at it, hint towards using long on extra1 and extra2 later. Signed-off-by: Luis Chamberlain --- fs/proc/proc_sysctl.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 5851eb5bc726..1df0beb50dbe 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1287,7 +1287,7 @@ static int insert_links(struct ctl_table_header *head) * __register_sysctl_table - register a leaf sysctl table * @set: Sysctl tree to register on * @path: The path to the directory the sysctl table is in. - * @table: the top-level table structure + * @table: the top-level table structure without any child * * Register a sysctl table hierarchy. @table should be a filled in ctl_table * array. A completely 0 filled entry terminates the table. @@ -1308,9 +1308,12 @@ static int insert_links(struct ctl_table_header *head) * proc_handler - the text handler routine (described below) * * extra1, extra2 - extra pointers usable by the proc handler routines + * XXX: we should eventually modify these to use long min / max [0] + * [0] https://lkml.kernel.org/87zgpte9o4@email.froward.int.ebiederm.org * * Leaf nodes in the sysctl tree will be represented by a single file - * under /proc; non-leaf nodes will be represented by directories. + * under /proc; non-leaf nodes (where child is not NULL) are not allowed, + * sysctl_check_table() verifies this. * * There must be a proc_handler routine for any terminal nodes. * Several default handlers are available to cover common cases - @@ -1352,7 +1355,7 @@ struct ctl_table_header *__register_sysctl_table( spin_lock(&sysctl_lock); dir = &set->dir; - /* Reference moved down the diretory tree get_subdir */ + /* Reference moved down the directory tree get_subdir */ dir->header.nreg++; spin_unlock(&sysctl_lock); @@ -1369,6 +1372,11 @@ struct ctl_table_header *__register_sysctl_table( if (namelen == 0) continue; + /* +* namelen ensures if name is "foo/bar/yay" only foo is +* registered first. We traverse as if using mkdir -p and +* return a ctl_dir for the last directory entry. +*/ dir = get_subdir(dir, name, namelen); if (IS_ERR(dir)) goto fail; -- 2.39.1
[apparmor] [PATCH 6/7] tty: simplify sysctl registration
register_sysctl_table() is a deprecated compatibility wrapper.
register_sysctl_init() can do the directory creation for you so just use
that
Signed-off-by: Luis Chamberlain
---
drivers/tty/tty_io.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 36fb945fdad4..766750e355ac 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3614,31 +3614,13 @@ static struct ctl_table tty_table[] = {
{ }
};
-static struct ctl_table tty_dir_table[] = {
- {
- .procname = "tty",
- .mode = 0555,
- .child = tty_table,
- },
- { }
-};
-
-static struct ctl_table tty_root_table[] = {
- {
- .procname = "dev",
- .mode = 0555,
- .child = tty_dir_table,
- },
- { }
-};
-
/*
* Ok, now we can initialize the rest of the tty devices and can count
* on memory allocations, interrupts etc..
*/
int __init tty_init(void)
{
- register_sysctl_table(tty_root_table);
+ register_sysctl_init("dev/tty", tty_table);
cdev_init(&tty_cdev, &tty_fops);
if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
--
2.39.1
[apparmor] [PATCH 00/11] sysctl: deprecate register_sysctl_paths()
As we trim down the insane kernel/sysctl.c large array and move sysctls out we're looking to optimize the way we do syctl registrations so we deal with just flat entries so to make the registration code much easier to maintain and so it does not recurse. In dealing with some of these things it reminded us that we will eventually get to the point of just passing in the ARRAY_SIZE() we want, to get there we should strive to move away from the older callers that do need the recursion. Turns out tons of users don't need the recursion already so we can start converting those over. register_sysctl_paths() can do recursion when its users have sysctl entries with directories and and then subdirectories with other entries. This *typically* was the use case in the large sysctl array in kernel/sysctl.c but as we trim that down we're phasing out the users that have subdirectories. This means that the code path that can do recursion is being mitigated over time and the code can be simplified as well. There are however many existing users of register_sysctl_paths() which don't need to deal with subdirectories, and register_sysctl() and the init version register_sysctl_init() (used when you don't care for the initial return value / need to de-register) can create subdirectories for you if you have no entries in between. So just convert these users to the simpler APIs and deprecate out register_sysctl_paths(). The next step after this is to to start deprecating slowly the callers of register_sysctl_table(). I'll send a out second batch for those that apply on top of this. Unless folks have an issue, I can offer to take these in the sysctl-next tree as we sweep these out, but I'm happy for folks to take these into their tree as well if they want. The only possible conflict would be timing, ie, if the final patch which removes the API call. That patch could also just wait another or two cycles later. Luis Chamberlain (11): proc_sysctl: update docs for __register_sysctl_table() proc_sysctl: move helper which creates required subdirectories sysctl: clarify register_sysctl_init() base directory order apparmor: simplify sysctls with register_sysctl_init() loadpin: simplify sysctls use with register_sysctl() yama: simplfy sysctls with register_sysctl() seccomp: simplify sysctls with register_sysctl_init() kernel: pid_namespace: simplify sysctls with register_sysctl() fs-verity: simplify sysctls with register_sysctl() csky: simplify alignment sysctl registration proc_sysctl: deprecate register_sysctl_paths() arch/csky/abiv1/alignment.c | 15 +- fs/proc/proc_sysctl.c | 95 +++-- fs/verity/signature.c | 9 +--- include/linux/sysctl.h | 11 - kernel/pid_namespace.c | 3 +- kernel/pid_sysctl.h | 3 +- kernel/seccomp.c| 15 +- security/apparmor/lsm.c | 8 +--- security/loadpin/loadpin.c | 8 +--- security/yama/yama_lsm.c| 8 +--- 10 files changed, 56 insertions(+), 119 deletions(-) -- 2.39.1
