On 2023-12-07 11:43:57+0100, Joel Granados wrote:
> Hey Thomas
>
> You have a couple of test bot issues for your 12/18 patch. Can you
> please address those for your next version.

I have these fixed locally, I assumed Luis would also pick them up
directly until I have a proper v2, properly should have communicated
that.

> On Mon, Dec 04, 2023 at 08:52:13AM +0100, Thomas Weißschuh wrote:
> > Problem description:
> >
> > The kernel contains a lot of struct ctl_table throught the tree.
> > These are very often 'static' definitions.
> > It would be good to make the tables unmodifiable by marking them "const"
> Here I would remove "It would be good to". Just state it: "Make the
> tables unmodifiable...."

Ack.

>
> > to avoid accidental or malicious modifications.
> > This is in line with a general effort to move as much data as possible
> > into .rodata. (See for example[0] and [1])

> If you could find more examples, it would make a better case.

I'll look for some. So far my constifications went in without them :-)

> >
> > Unfortunately the tables can not be made const right now because the
> > core registration functions expect mutable tables.
> >
> > This is for two main reasons:
> >
> > 1) sysctl_{set,clear}_perm_empty_ctl_header in the sysctl core modify
> > the table.
> > 2) The table is passed to the handler function as a non-const pointer.
> >
> > This series migrates the core and all handlers.

> awesome!
>
> >
> > Structure of the series:
> >
> > Patch 1-3: Cleanup patches
> > Patch 4-7: Non-logic preparation patches
> > Patch 8: Preparation patch changing a bit of logic
> > Patch 9-12: Treewide changes to handler function signature
> > Patch 13-14: Adaption of the sysctl core implementation
> > Patch 15: Adaption of the sysctl core interface
> > Patch 16: New entry for checkpatch
> > Patch 17-18: Constification of existing "struct ctl_table"s
> >
> > Tested by booting and with the sysctl selftests on x86.
> >
> > Note:
> >
> > This is intentionally sent only to a small number of people as I'd like
> > to get some more sysctl core-maintainer feedback before sending this to
> > essentially everybody.

> When you do send it to the broader audience, you should chunk up your big
> patches (12/18 and 11/18) and this is why:
> 1. To avoid mail rejections from lists:
> You have to tell a lot of people about the changes in one mail. That
> will make mail header too big for some lists and it will be rejected.
> This happened to me with [3]
> 2. Avoid being rejected for the wrong reasons :)
> Maintainers are busy ppl and sending them a set with so many files
> may elicit a rejection on the grounds that it involves too many
> subsystems at the same time.
> I suggest you chunk it up with directories in mind. Something similar to
> what I did for [4] where I divided stuff that when for fs/*, kernel/*,
> net/*, arch/* and drivers/*. That will complicate your patch a tad
> because you have to ensure that the tree can be compiled/run for every
> commit. But it will pay off once you push it to the broader public.

This will break bisections. All function signatures need to be switched
in one step. I would strongly like to avoid introducing broken commits.

The fact that these big commits have no functional changes at all makes
me hope it can work.

2023-12-07 19:23:42

by Thomas Weißschuh

[permalink] [raw]

Subject: Re: [PATCH v2 00/18] sysctl: constify sysctl ctl_tables

On 2023-12-07 12:19:57+0100, Joel Granados wrote:
> On Tue, Dec 05, 2023 at 06:16:53PM +0100, Thomas Weißschuh wrote:
> > Hi Luis, Joel,
> >
> > On 2023-12-05 09:04:08+0100, Thomas Weißschuh wrote:
> > > On 2023-12-04 21:50:14-0800, Luis Chamberlain wrote:
> > > > On Mon, Dec 04, 2023 at 08:52:13AM +0100, Thomas Weißschuh wrote:
> > > > > Tested by booting and with the sysctl selftests on x86.
> > > >
> > > > Can I trouble you to rebase on sysctl-next?
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-next
> > >
> > > Will do.
> >
> > The rebased series is now available at
> > https://git.sr.ht/~t-8ch/linux b4/const-sysctl
> >
> > Nothing much has changed in contrast to v2.
> > The only functional change so far is the initialization of
> > ctl_table_header::type in init_header().
> >
> > I'll wait for Joels and maybe some more reviews before resending it.
> >
> > > [..]
> >
> > For the future I think it would make sense to combine the tree-wide constification
> > of the structs with the removal of the sentinel values.

> I don't see how these two would fit. And this is why:
> 1. The "remove sentinel" stuff is almost done. With the sets going into
> 6.7 we would only be missing everything under net/*. So you would not
> be able to combine them (except for the net stuff)
> 2. The motivation for the two sets is differnt. This would confuse
> rather than simplify the process.
> 3. In order to introduce the const stuff we would have to go through
> another round of "convincing" which can potentially derail the
> "remove sentinel" stuff.

Good reasons, especially 1).

> I would *not* like to combine them. I think the const set can stand on
> its own.

It was more about a process optimization. If somebody has to touch each
sysctl table anyway and test the changes, doing so for both series would
be easier for the sysctl and subsystem maintainers.

But alas, it seems I have to do it myself and can't heap it onto your
pile :-)

> >
> > This would reduce the impacts of the maintainers.

2023-12-07 19:29:30

by Thomas Weißschuh

Hi Joel,

On 2023-12-21 13:09:19+0100, Joel Granados wrote:
> I had some available cycles today and wanted to look at the patch that
> you sent. I could not apply it on top of 6.7-rc6. Have you thought about
> taking this out of your "constification" series and posting it as
> something that comes before the const stuff?

Yes, I am planning to submit this standalone. But I need to do some more
investigation to properly handle the existing comments.

FYI this out-of-series patch is meant to be applied on top of the rest
of the series.

Another question:

When did you write this mail?

The Date header says it's from the 21st but I only received it today
(the 23rd). All other mail headers also say that it was only sent on
23rd. The same seems to happen with your other mails, too.

Thomas

> On Wed, Dec 06, 2023 at 06:53:10AM +0100, Thomas Weißschuh wrote:
> > On 2023-12-05 14:50:01-0800, Luis Chamberlain wrote:
> > > On Tue, Dec 5, 2023 at 2:41 PM Thomas Weißschuh <[email protected]> wrote:
> > > >
> > > > On 2023-12-05 14:33:38-0800, Luis Chamberlain wrote:
> > > > > On Mon, Dec 04, 2023 at 08:52:26AM +0100, Thomas Weißschuh wrote:
> > > > > > @@ -231,7 +231,8 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > > > > > return -EROFS;
> > > > > >
> > > > > > /* Am I creating a permanently empty directory? */
> > > > > > - if (sysctl_is_perm_empty_ctl_header(header)) {
> > > > > > + if (header->ctl_table == sysctl_mount_point ||
> > > > > > + sysctl_is_perm_empty_ctl_header(header)) {
> > > > > > if (!RB_EMPTY_ROOT(&dir->root))
> > > > > > return -EINVAL;
> > > > > > sysctl_set_perm_empty_ctl_header(dir_h);
> > > > >
> > > > > While you're at it.
> > > >
> > > > This hunk is completely gone in v3/the code that you merged.
> > >
> > > It is worse in that it is not obvious:
> > >
> > > + if (table == sysctl_mount_point)
> > > + sysctl_set_perm_empty_ctl_header(head);
> > >
> > > > Which kind of unsafety do you envision here?
> > >
> > > Making the code obvious during patch review hy this is needed /
> > > special, and if we special case this, why not remove enum, and make it
> > > specific to only that one table. The catch is that it is not
> > > immediately obvious that we actually call
> > > sysctl_set_perm_empty_ctl_header() in other places, and it begs the
> > > question if this can be cleaned up somehow.
> >
> > Making it specific won't work because the flag needs to be transferred
> > from the leaf table to the table representing the directory.
> >
> > What do you think of the aproach taken in the attached patch?
> > (On top of current sysctl-next, including my series)
> >
> > Note: Current sysctl-next ist still based on v6.6.
>
> > From 2fb9887fb2a5024c2620f2d694bc6dcc32afde67 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= <[email protected]>
> > Date: Wed, 6 Dec 2023 06:17:22 +0100
> > Subject: [PATCH] sysctl: simplify handling of permanently empty directories
> >
> > ---
> > fs/proc/proc_sysctl.c | 76 +++++++++++++++++++-----------------------
> > include/linux/sysctl.h | 13 ++------
> > 2 files changed, 36 insertions(+), 53 deletions(-)
> >
> > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > index c92e9b972ada..c4d6d09b0e68 100644
> > --- a/fs/proc/proc_sysctl.c
> > +++ b/fs/proc/proc_sysctl.c
> > @@ -17,6 +17,7 @@
> > #include <linux/bpf-cgroup.h>
> > #include <linux/mount.h>
> > #include <linux/kmemleak.h>
> > +#include <linux/cleanup.h>
> > #include "internal.h"
> >
> > #define list_for_each_table_entry(entry, header) \
> > @@ -29,32 +30,6 @@ static const struct inode_operations proc_sys_inode_operations;
> > static const struct file_operations proc_sys_dir_file_operations;
> > static const struct inode_operations proc_sys_dir_operations;
> >
> > -/* Support for permanently empty directories */
> > -static const struct ctl_table sysctl_mount_point[] = {
> > - { }
> > -};
> > -
> > -/**
> > - * register_sysctl_mount_point() - registers a sysctl mount point
> > - * @path: path for the mount point
> > - *
> > - * Used to create a permanently empty directory to serve as mount point.
> > - * There are some subtle but important permission checks this allows in the
> > - * case of unprivileged mounts.
> > - */
> > -struct ctl_table_header *register_sysctl_mount_point(const char *path)
> > -{
> > - return register_sysctl(path, sysctl_mount_point);
> > -}
> > -EXPORT_SYMBOL(register_sysctl_mount_point);
> > -
> > -#define sysctl_is_perm_empty_ctl_header(hptr) \
> > - (hptr->type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY)
> > -#define sysctl_set_perm_empty_ctl_header(hptr) \
> > - (hptr->type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY)
> > -#define sysctl_clear_perm_empty_ctl_header(hptr) \
> > - (hptr->type = SYSCTL_TABLE_TYPE_DEFAULT)
> > -
> > void proc_sys_poll_notify(struct ctl_table_poll *poll)
> > {
> > if (!poll)
> > @@ -199,8 +174,6 @@ static void init_header(struct ctl_table_header *head,
> > head->set = set;
> > head->parent = NULL;
> > head->node = node;
> > - if (table == sysctl_mount_point)
> > - sysctl_set_perm_empty_ctl_header(head);
> > INIT_HLIST_HEAD(&head->inodes);
> > if (node) {
> > const struct ctl_table *entry;
> > @@ -228,17 +201,9 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> >
> >
> > /* Is this a permanently empty directory? */
> > - if (sysctl_is_perm_empty_ctl_header(dir_h))
> > + if (dir->permanently_empty)
> > return -EROFS;
> >
> > - /* Am I creating a permanently empty directory? */
> > - if (header->ctl_table_size > 0 &&
> > - sysctl_is_perm_empty_ctl_header(header)) {
> > - if (!RB_EMPTY_ROOT(&dir->root))
> > - return -EINVAL;
> > - sysctl_set_perm_empty_ctl_header(dir_h);
> > - }
> > -
> > dir_h->nreg++;
> > header->parent = dir;
> > err = insert_links(header);
> > @@ -254,8 +219,6 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > erase_header(header);
> > put_links(header);
> > fail_links:
> > - if (header->ctl_table == sysctl_mount_point)
> > - sysctl_clear_perm_empty_ctl_header(dir_h);
> > header->parent = NULL;
> > drop_sysctl_table(dir_h);
> > return err;
> > @@ -442,6 +405,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
> > struct ctl_table_header *head, const struct ctl_table *table)
> > {
> > struct ctl_table_root *root = head->root;
> > + struct ctl_dir *ctl_dir;
> > struct inode *inode;
> > struct proc_inode *ei;
> >
> > @@ -475,7 +439,9 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
> > inode->i_mode |= S_IFDIR;
> > inode->i_op = &proc_sys_dir_operations;
> > inode->i_fop = &proc_sys_dir_file_operations;
> > - if (sysctl_is_perm_empty_ctl_header(head))
> > +
> > + ctl_dir = container_of(head, struct ctl_dir, header);
> > + if (ctl_dir->permanently_empty)
> > make_empty_dir_inode(inode);
> > }
> >
> > @@ -1214,8 +1180,7 @@ static bool get_links(struct ctl_dir *dir,
> > struct ctl_table_header *tmp_head;
> > const struct ctl_table *entry, *link;
> >
> > - if (header->ctl_table_size == 0 ||
> > - sysctl_is_perm_empty_ctl_header(header))
> > + if (header->ctl_table_size == 0 || dir->permanently_empty)
> > return true;
> >
> > /* Are there links available for every entry in table? */
> > @@ -1536,6 +1501,33 @@ void unregister_sysctl_table(struct ctl_table_header * header)
> > }
> > EXPORT_SYMBOL(unregister_sysctl_table);
> >
> > +/**
> > + * register_sysctl_mount_point() - registers a sysctl mount point
> > + * @path: path for the mount point
> > + *
> > + * Used to create a permanently empty directory to serve as mount point.
> > + * There are some subtle but important permission checks this allows in the
> > + * case of unprivileged mounts.
> > + */
> > +struct ctl_table_header *register_sysctl_mount_point(const char *path)
> > +{
> > + struct ctl_dir *dir = sysctl_mkdir_p(&sysctl_table_root.default_set.dir, path);
> > +
> > + if (IS_ERR(dir))
> > + return NULL;
> > +
> > + guard(spinlock)(&sysctl_lock);
> > +
> > + if (!RB_EMPTY_ROOT(&dir->root)) {
> > + drop_sysctl_table(&dir->header);
> > + return NULL;
> > + }
> > +
> > + dir->permanently_empty = true;
> > + return &dir->header;
> > +}
> > +EXPORT_SYMBOL(register_sysctl_mount_point);
> > +
> > void setup_sysctl_set(struct ctl_table_set *set,
> > struct ctl_table_root *root,
> > int (*is_seen)(struct ctl_table_set *))
> > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > index 7c96d5abafc7..329e68d484ed 100644
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -177,23 +177,14 @@ struct ctl_table_header {
> > struct ctl_dir *parent;
> > struct ctl_node *node;
> > struct hlist_head inodes; /* head for proc_inode->sysctl_inodes */
> > - /**
> > - * enum type - Enumeration to differentiate between ctl target types
> > - * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
> > - * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
> > - * empty directory target to serve
> > - * as mount point.
> > - */
> > - enum {
> > - SYSCTL_TABLE_TYPE_DEFAULT,
> > - SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> > - } type;
> > };
> >
> > struct ctl_dir {
> > /* Header must be at the start of ctl_dir */
> > struct ctl_table_header header;
> > struct rb_root root;
> > + /* Permanently empty directory target to serve as mount point. */
> > + bool permanently_empty;
> > };
> >
> > struct ctl_table_set {
> >
> > base-commit: a6fd07f80ab7bd94edb4d56c35e61117ffb9957e
> > prerequisite-patch-id: 0000000000000000000000000000000000000000
> > prerequisite-patch-id: 13932e9add940cb65c71e04b5efdfcd3622fd27e
> > prerequisite-patch-id: 2e4d88f7b8aaa805598f0e87a3ea726825bb4264
> > prerequisite-patch-id: 674a680d9cb138cd34cfd0e1a4ec3a5d1c220078
> > prerequisite-patch-id: e27c92582aa20b1dfb122c172b336dbaf9d6508a
> > prerequisite-patch-id: 9b409a34ab6a4d8d8c5225ba9a72db3116e3c8b3
> > prerequisite-patch-id: 86ff15a81d850ebda16bb707491251f4b705e4fd
> > prerequisite-patch-id: b7ab65512ac9acfb2dd482b0271b399467afc56d
> > prerequisite-patch-id: 0354922fbf2508a89f3e9d9a4e274fc98deb2e93
> > prerequisite-patch-id: b71389e82026ffc19cbb717bba1a014ad6cab6da
> > prerequisite-patch-id: fbb0201f89bf6c41d0585af867bdeec8d51649b2
> > prerequisite-patch-id: e3b4b5b69b4eadf87ed97beb8c03a471e7628cb9
> > prerequisite-patch-id: 3fbc9745cf3f28872b3e63f6d1f6e2fd7598be8a
> > prerequisite-patch-id: ba2b190c2e54cfb505a282e688c2222712f0acd7
> > prerequisite-patch-id: 47e5ca730748bb7bf9248a9e711045d8c1028199
> > prerequisite-patch-id: dcd9f87f00290d2f9be83e404f8883eb90c5fb1c
> > prerequisite-patch-id: d4629be1a61585ab821da2d2850f246761f72f25
> > prerequisite-patch-id: f740190f4b94e57cbf3659f220d94483713341a1
> > prerequisite-patch-id: 301c2e530e2af4568267e19247d4a49ac2a9871d
> > --
> > 2.43.0
> >
>
>
> --
>
> Joel Granados

2023-12-24 18:51:52

by Joel Granados

[permalink] [raw]

Subject: Re: [PATCH v2 13/18] sysctl: move sysctl type to ctl_table_header

On Sat, Dec 23, 2023 at 02:04:27PM +0100, Thomas Weißschuh wrote:
> Hi Joel,
>
> On 2023-12-21 13:09:19+0100, Joel Granados wrote:
> > I had some available cycles today and wanted to look at the patch that
> > you sent. I could not apply it on top of 6.7-rc6. Have you thought about
> > taking this out of your "constification" series and posting it as
> > something that comes before the const stuff?
>
> Yes, I am planning to submit this standalone. But I need to do some more
> investigation to properly handle the existing comments.
Understood.

>
> FYI this out-of-series patch is meant to be applied on top of the rest
> of the series.
Ok.

>
>
> Another question:
>
> When did you write this mail?
>
> The Date header says it's from the 21st but I only received it today
> (the 23rd). All other mail headers also say that it was only sent on
> 23rd. The same seems to happen with your other mails, too.
Sometimes I do the review even if I don't have connection. Everything
goes out once I get back on the network.

So there is the timestamp of when I "sent" but was on standby in my
outgoing mails 21st. And then the actual sending occurred some days
later when I was back on the network.

>
> Thomas
>
> > On Wed, Dec 06, 2023 at 06:53:10AM +0100, Thomas Weißschuh wrote:
> > > On 2023-12-05 14:50:01-0800, Luis Chamberlain wrote:
> > > > On Tue, Dec 5, 2023 at 2:41 PM Thomas Weißschuh <[email protected]> wrote:
> > > > >
> > > > > On 2023-12-05 14:33:38-0800, Luis Chamberlain wrote:
> > > > > > On Mon, Dec 04, 2023 at 08:52:26AM +0100, Thomas Weißschuh wrote:
> > > > > > > @@ -231,7 +231,8 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > > > > > > return -EROFS;
> > > > > > >
> > > > > > > /* Am I creating a permanently empty directory? */
> > > > > > > - if (sysctl_is_perm_empty_ctl_header(header)) {
> > > > > > > + if (header->ctl_table == sysctl_mount_point ||
> > > > > > > + sysctl_is_perm_empty_ctl_header(header)) {
> > > > > > > if (!RB_EMPTY_ROOT(&dir->root))
> > > > > > > return -EINVAL;
> > > > > > > sysctl_set_perm_empty_ctl_header(dir_h);
> > > > > >
> > > > > > While you're at it.
> > > > >
> > > > > This hunk is completely gone in v3/the code that you merged.
> > > >
> > > > It is worse in that it is not obvious:
> > > >
> > > > + if (table == sysctl_mount_point)
> > > > + sysctl_set_perm_empty_ctl_header(head);
> > > >
> > > > > Which kind of unsafety do you envision here?
> > > >
> > > > Making the code obvious during patch review hy this is needed /
> > > > special, and if we special case this, why not remove enum, and make it
> > > > specific to only that one table. The catch is that it is not
> > > > immediately obvious that we actually call
> > > > sysctl_set_perm_empty_ctl_header() in other places, and it begs the
> > > > question if this can be cleaned up somehow.
> > >
> > > Making it specific won't work because the flag needs to be transferred
> > > from the leaf table to the table representing the directory.
> > >
> > > What do you think of the aproach taken in the attached patch?
> > > (On top of current sysctl-next, including my series)
> > >
> > > Note: Current sysctl-next ist still based on v6.6.
> >
> > > From 2fb9887fb2a5024c2620f2d694bc6dcc32afde67 Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= <[email protected]>
> > > Date: Wed, 6 Dec 2023 06:17:22 +0100
> > > Subject: [PATCH] sysctl: simplify handling of permanently empty directories
> > >
> > > ---
> > > fs/proc/proc_sysctl.c | 76 +++++++++++++++++++-----------------------
> > > include/linux/sysctl.h | 13 ++------
> > > 2 files changed, 36 insertions(+), 53 deletions(-)
> > >
> > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > > index c92e9b972ada..c4d6d09b0e68 100644
> > > --- a/fs/proc/proc_sysctl.c
> > > +++ b/fs/proc/proc_sysctl.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/bpf-cgroup.h>
> > > #include <linux/mount.h>
> > > #include <linux/kmemleak.h>
> > > +#include <linux/cleanup.h>
> > > #include "internal.h"
> > >
> > > #define list_for_each_table_entry(entry, header) \
> > > @@ -29,32 +30,6 @@ static const struct inode_operations proc_sys_inode_operations;
> > > static const struct file_operations proc_sys_dir_file_operations;
> > > static const struct inode_operations proc_sys_dir_operations;
> > >
> > > -/* Support for permanently empty directories */
> > > -static const struct ctl_table sysctl_mount_point[] = {
> > > - { }
> > > -};
> > > -
> > > -/**
> > > - * register_sysctl_mount_point() - registers a sysctl mount point
> > > - * @path: path for the mount point
> > > - *
> > > - * Used to create a permanently empty directory to serve as mount point.
> > > - * There are some subtle but important permission checks this allows in the
> > > - * case of unprivileged mounts.
> > > - */
> > > -struct ctl_table_header *register_sysctl_mount_point(const char *path)
> > > -{
> > > - return register_sysctl(path, sysctl_mount_point);
> > > -}
> > > -EXPORT_SYMBOL(register_sysctl_mount_point);
> > > -
> > > -#define sysctl_is_perm_empty_ctl_header(hptr) \
> > > - (hptr->type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY)
> > > -#define sysctl_set_perm_empty_ctl_header(hptr) \
> > > - (hptr->type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY)
> > > -#define sysctl_clear_perm_empty_ctl_header(hptr) \
> > > - (hptr->type = SYSCTL_TABLE_TYPE_DEFAULT)
> > > -
> > > void proc_sys_poll_notify(struct ctl_table_poll *poll)
> > > {
> > > if (!poll)
> > > @@ -199,8 +174,6 @@ static void init_header(struct ctl_table_header *head,
> > > head->set = set;
> > > head->parent = NULL;
> > > head->node = node;
> > > - if (table == sysctl_mount_point)
> > > - sysctl_set_perm_empty_ctl_header(head);
> > > INIT_HLIST_HEAD(&head->inodes);
> > > if (node) {
> > > const struct ctl_table *entry;
> > > @@ -228,17 +201,9 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > >
> > >
> > > /* Is this a permanently empty directory? */
> > > - if (sysctl_is_perm_empty_ctl_header(dir_h))
> > > + if (dir->permanently_empty)
> > > return -EROFS;
> > >
> > > - /* Am I creating a permanently empty directory? */
> > > - if (header->ctl_table_size > 0 &&
> > > - sysctl_is_perm_empty_ctl_header(header)) {
> > > - if (!RB_EMPTY_ROOT(&dir->root))
> > > - return -EINVAL;
> > > - sysctl_set_perm_empty_ctl_header(dir_h);
> > > - }
> > > -
> > > dir_h->nreg++;
> > > header->parent = dir;
> > > err = insert_links(header);
> > > @@ -254,8 +219,6 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > > erase_header(header);
> > > put_links(header);
> > > fail_links:
> > > - if (header->ctl_table == sysctl_mount_point)
> > > - sysctl_clear_perm_empty_ctl_header(dir_h);
> > > header->parent = NULL;
> > > drop_sysctl_table(dir_h);
> > > return err;
> > > @@ -442,6 +405,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
> > > struct ctl_table_header *head, const struct ctl_table *table)
> > > {
> > > struct ctl_table_root *root = head->root;
> > > + struct ctl_dir *ctl_dir;
> > > struct inode *inode;
> > > struct proc_inode *ei;
> > >
> > > @@ -475,7 +439,9 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
> > > inode->i_mode |= S_IFDIR;
> > > inode->i_op = &proc_sys_dir_operations;
> > > inode->i_fop = &proc_sys_dir_file_operations;
> > > - if (sysctl_is_perm_empty_ctl_header(head))
> > > +
> > > + ctl_dir = container_of(head, struct ctl_dir, header);
> > > + if (ctl_dir->permanently_empty)
> > > make_empty_dir_inode(inode);
> > > }
> > >
> > > @@ -1214,8 +1180,7 @@ static bool get_links(struct ctl_dir *dir,
> > > struct ctl_table_header *tmp_head;
> > > const struct ctl_table *entry, *link;
> > >
> > > - if (header->ctl_table_size == 0 ||
> > > - sysctl_is_perm_empty_ctl_header(header))
> > > + if (header->ctl_table_size == 0 || dir->permanently_empty)
> > > return true;
> > >
> > > /* Are there links available for every entry in table? */
> > > @@ -1536,6 +1501,33 @@ void unregister_sysctl_table(struct ctl_table_header * header)
> > > }
> > > EXPORT_SYMBOL(unregister_sysctl_table);
> > >
> > > +/**
> > > + * register_sysctl_mount_point() - registers a sysctl mount point
> > > + * @path: path for the mount point
> > > + *
> > > + * Used to create a permanently empty directory to serve as mount point.
> > > + * There are some subtle but important permission checks this allows in the
> > > + * case of unprivileged mounts.
> > > + */
> > > +struct ctl_table_header *register_sysctl_mount_point(const char *path)
> > > +{
> > > + struct ctl_dir *dir = sysctl_mkdir_p(&sysctl_table_root.default_set.dir, path);
> > > +
> > > + if (IS_ERR(dir))
> > > + return NULL;
> > > +
> > > + guard(spinlock)(&sysctl_lock);
> > > +
> > > + if (!RB_EMPTY_ROOT(&dir->root)) {
> > > + drop_sysctl_table(&dir->header);
> > > + return NULL;
> > > + }
> > > +
> > > + dir->permanently_empty = true;
> > > + return &dir->header;
> > > +}
> > > +EXPORT_SYMBOL(register_sysctl_mount_point);
> > > +
> > > void setup_sysctl_set(struct ctl_table_set *set,
> > > struct ctl_table_root *root,
> > > int (*is_seen)(struct ctl_table_set *))
> > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > > index 7c96d5abafc7..329e68d484ed 100644
> > > --- a/include/linux/sysctl.h
> > > +++ b/include/linux/sysctl.h
> > > @@ -177,23 +177,14 @@ struct ctl_table_header {
> > > struct ctl_dir *parent;
> > > struct ctl_node *node;
> > > struct hlist_head inodes; /* head for proc_inode->sysctl_inodes */
> > > - /**
> > > - * enum type - Enumeration to differentiate between ctl target types
> > > - * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
> > > - * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
> > > - * empty directory target to serve
> > > - * as mount point.
> > > - */
> > > - enum {
> > > - SYSCTL_TABLE_TYPE_DEFAULT,
> > > - SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> > > - } type;
> > > };
> > >
> > > struct ctl_dir {
> > > /* Header must be at the start of ctl_dir */
> > > struct ctl_table_header header;
> > > struct rb_root root;
> > > + /* Permanently empty directory target to serve as mount point. */
> > > + bool permanently_empty;
> > > };
> > >
> > > struct ctl_table_set {
> > >
> > > base-commit: a6fd07f80ab7bd94edb4d56c35e61117ffb9957e
> > > prerequisite-patch-id: 0000000000000000000000000000000000000000
> > > prerequisite-patch-id: 13932e9add940cb65c71e04b5efdfcd3622fd27e
> > > prerequisite-patch-id: 2e4d88f7b8aaa805598f0e87a3ea726825bb4264
> > > prerequisite-patch-id: 674a680d9cb138cd34cfd0e1a4ec3a5d1c220078
> > > prerequisite-patch-id: e27c92582aa20b1dfb122c172b336dbaf9d6508a
> > > prerequisite-patch-id: 9b409a34ab6a4d8d8c5225ba9a72db3116e3c8b3
> > > prerequisite-patch-id: 86ff15a81d850ebda16bb707491251f4b705e4fd
> > > prerequisite-patch-id: b7ab65512ac9acfb2dd482b0271b399467afc56d
> > > prerequisite-patch-id: 0354922fbf2508a89f3e9d9a4e274fc98deb2e93
> > > prerequisite-patch-id: b71389e82026ffc19cbb717bba1a014ad6cab6da
> > > prerequisite-patch-id: fbb0201f89bf6c41d0585af867bdeec8d51649b2
> > > prerequisite-patch-id: e3b4b5b69b4eadf87ed97beb8c03a471e7628cb9
> > > prerequisite-patch-id: 3fbc9745cf3f28872b3e63f6d1f6e2fd7598be8a
> > > prerequisite-patch-id: ba2b190c2e54cfb505a282e688c2222712f0acd7
> > > prerequisite-patch-id: 47e5ca730748bb7bf9248a9e711045d8c1028199
> > > prerequisite-patch-id: dcd9f87f00290d2f9be83e404f8883eb90c5fb1c
> > > prerequisite-patch-id: d4629be1a61585ab821da2d2850f246761f72f25
> > > prerequisite-patch-id: f740190f4b94e57cbf3659f220d94483713341a1
> > > prerequisite-patch-id: 301c2e530e2af4568267e19247d4a49ac2a9871d
> > > --
> > > 2.43.0
> > >
> >
> >
> > --
> >
> > Joel Granados
>
>

--

Joel Granados

Attachments:

(No filename) (12.13 kB)
signature.asc (673.00 B)
Download all attachments