The argument is never used and can be removed.
In a future commit the sysctl core will only use
"const struct ctl_table". Removing it here is a preparation for this
consitifcation.
The patch was created with the following coccinelle script:
@@
identifier func, head, table, uid, gid;
@@
void func(
struct ctl_table_header *head,
- struct ctl_table *table,
kuid_t *uid, kgid_t *gid)
{ ... }
The single changed location was validate through manual inspection and
compilation.
In addition, a search for 'set_ownership' was done over the full tree to
look for places that were missed by coccinelle.
None were found.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
Changes in v2:
- Rework commit message
- Mention potential conflict with upcoming per-namespace kernel.pid_max
sysctl
- Delete unused parameter table
- Link to v1: https://lore.kernel.org/r/[email protected]
---
The patch is meant to be merged via the sysctl tree.
There is an upcoming series that will introduce a new implementation of
set_ownership which would need to be adapted [0].
The adaption would be trivial as the 'table' parameter also unused
there.
This change was originally part of the sysctl-const series [1].
To slim down that series and reduce the message load on other
maintainers to a minimumble, submit this patch on its own.
[0] https://lore.kernel.org/lkml/[email protected]/
[1] https://lore.kernel.org/lkml/[email protected]/
---
include/linux/sysctl.h | 1 -
net/sysctl_net.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index ee7d33b89e9e..60333a6b9370 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -205,7 +205,6 @@ struct ctl_table_root {
struct ctl_table_set default_set;
struct ctl_table_set *(*lookup)(struct ctl_table_root *root);
void (*set_ownership)(struct ctl_table_header *head,
- struct ctl_table *table,
kuid_t *uid, kgid_t *gid);
int (*permissions)(struct ctl_table_header *head, struct ctl_table *table);
};
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index 051ed5f6fc93..a0a7a79991f9 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -54,7 +54,6 @@ static int net_ctl_permissions(struct ctl_table_header *head,
}
static void net_ctl_set_ownership(struct ctl_table_header *head,
- struct ctl_table *table,
kuid_t *uid, kgid_t *gid)
{
struct net *net = container_of(head->set, struct net, sysctls);
---
base-commit: ffd2cb6b718e189e7e2d5d0c19c25611f92e061a
change-id: 20231226-sysctl-const-ownership-ff75e67b4eea
Best regards,
--
Thomas Weißschuh <[email protected]>
Hey Thomas
Did you forget to compile? I'm seeing the following error when I
compile:
...
../fs/proc/proc_sysctl.c: In function ‘proc_sys_make_inode’:
../fs/proc/proc_sysctl.c:483:43: error: passing argument 2 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
| ^~~~~
| |
| struct ctl_table *
../fs/proc/proc_sysctl.c:483:43: note: expected ‘kuid_t *’ but argument is of type ‘struct ctl_table *’
../fs/proc/proc_sysctl.c:483:50: error: passing argument 3 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
| ^~~~~~~~~~~~~
| |
| kuid_t *
../fs/proc/proc_sysctl.c:483:50: note: expected ‘kgid_t *’ but argument is of type ‘kuid_t *’
../fs/proc/proc_sysctl.c:483:17: error: too many arguments to function ‘root->set_ownership’
483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
| ^~~~
cc1: some warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:243: fs/proc/proc_sysctl.o] Error 1
CC fs/xfs/libxfs/xfs_dir2_node.o
make[5]: *** Waiting for unfinished jobs....
...
I'm guessing its just a matter of removing the table arg from
proc_sys_make_inode?
Best
On Fri, Feb 23, 2024 at 04:50:00PM +0100, Thomas Weißschuh wrote:
> The argument is never used and can be removed.
>
> In a future commit the sysctl core will only use
> "const struct ctl_table". Removing it here is a preparation for this
> consitifcation.
>
> The patch was created with the following coccinelle script:
>
> @@
> identifier func, head, table, uid, gid;
> @@
>
> void func(
> struct ctl_table_header *head,
> - struct ctl_table *table,
> kuid_t *uid, kgid_t *gid)
> { ... }
>
> The single changed location was validate through manual inspection and
> compilation.
>
> In addition, a search for 'set_ownership' was done over the full tree to
> look for places that were missed by coccinelle.
> None were found.
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
> ---
> Changes in v2:
> - Rework commit message
> - Mention potential conflict with upcoming per-namespace kernel.pid_max
> sysctl
> - Delete unused parameter table
> - Link to v1: https://lore.kernel.org/r/[email protected]
> ---
> The patch is meant to be merged via the sysctl tree.
>
> There is an upcoming series that will introduce a new implementation of
> .set_ownership which would need to be adapted [0].
> The adaption would be trivial as the 'table' parameter also unused
> there.
>
> This change was originally part of the sysctl-const series [1].
> To slim down that series and reduce the message load on other
> maintainers to a minimumble, submit this patch on its own.
>
> [0] https://lore.kernel.org/lkml/[email protected]/
> [1] https://lore.kernel.org/lkml/[email protected]/
> ---
> include/linux/sysctl.h | 1 -
> net/sysctl_net.c | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index ee7d33b89e9e..60333a6b9370 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -205,7 +205,6 @@ struct ctl_table_root {
> struct ctl_table_set default_set;
> struct ctl_table_set *(*lookup)(struct ctl_table_root *root);
> void (*set_ownership)(struct ctl_table_header *head,
> - struct ctl_table *table,
> kuid_t *uid, kgid_t *gid);
> int (*permissions)(struct ctl_table_header *head, struct ctl_table *table);
> };
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index 051ed5f6fc93..a0a7a79991f9 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -54,7 +54,6 @@ static int net_ctl_permissions(struct ctl_table_header *head,
> }
>
> static void net_ctl_set_ownership(struct ctl_table_header *head,
> - struct ctl_table *table,
> kuid_t *uid, kgid_t *gid)
> {
> struct net *net = container_of(head->set, struct net, sysctls);
>
> ---
> base-commit: ffd2cb6b718e189e7e2d5d0c19c25611f92e061a
> change-id: 20231226-sysctl-const-ownership-ff75e67b4eea
>
> Best regards,
> --
> Thomas Weißschuh <[email protected]>
>
--
Joel Granados
On 2024-03-15 16:41:34+0100, Joel Granados wrote:
> Hey Thomas
>
> Did you forget to compile? I'm seeing the following error when I
> compile:
Welp...
> ...
> ../fs/proc/proc_sysctl.c: In function ‘proc_sys_make_inode’:
> ../fs/proc/proc_sysctl.c:483:43: error: passing argument 2 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> | ^~~~~
> | |
> | struct ctl_table *
> ../fs/proc/proc_sysctl.c:483:43: note: expected ‘kuid_t *’ but argument is of type ‘struct ctl_table *’
> ../fs/proc/proc_sysctl.c:483:50: error: passing argument 3 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> | ^~~~~~~~~~~~~
> | |
> | kuid_t *
> ../fs/proc/proc_sysctl.c:483:50: note: expected ‘kgid_t *’ but argument is of type ‘kuid_t *’
> ../fs/proc/proc_sysctl.c:483:17: error: too many arguments to function ‘root->set_ownership’
> 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> | ^~~~
> cc1: some warnings being treated as errors
> make[5]: *** [../scripts/Makefile.build:243: fs/proc/proc_sysctl.o] Error 1
> CC fs/xfs/libxfs/xfs_dir2_node.o
> make[5]: *** Waiting for unfinished jobs....
> ...
>
> I'm guessing its just a matter of removing the table arg from
> proc_sys_make_inode?
Yes, for this error it's correct.
There are also new implementors of the set_ownership callback in ipc/
which need to be adapted.
I'll send a new revision.
Thomas
On Fri, Mar 15, 2024 at 04:56:54PM +0100, Thomas Weißschuh wrote:
> On 2024-03-15 16:41:34+0100, Joel Granados wrote:
> > Hey Thomas
> >
> > Did you forget to compile? I'm seeing the following error when I
> > compile:
>
> Welp...
>
> > ...
> > ../fs/proc/proc_sysctl.c: In function ‘proc_sys_make_inode’:
> > ../fs/proc/proc_sysctl.c:483:43: error: passing argument 2 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> > 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> > | ^~~~~
> > | |
> > | struct ctl_table *
> > ../fs/proc/proc_sysctl.c:483:43: note: expected ‘kuid_t *’ but argument is of type ‘struct ctl_table *’
> > ../fs/proc/proc_sysctl.c:483:50: error: passing argument 3 of ‘root->set_ownership’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> > 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> > | ^~~~~~~~~~~~~
> > | |
> > | kuid_t *
> > ../fs/proc/proc_sysctl.c:483:50: note: expected ‘kgid_t *’ but argument is of type ‘kuid_t *’
> > ../fs/proc/proc_sysctl.c:483:17: error: too many arguments to function ‘root->set_ownership’
> > 483 | root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> > | ^~~~
> > cc1: some warnings being treated as errors
> > make[5]: *** [../scripts/Makefile.build:243: fs/proc/proc_sysctl.o] Error 1
> > CC fs/xfs/libxfs/xfs_dir2_node.o
> > make[5]: *** Waiting for unfinished jobs....
> > ...
> >
> > I'm guessing its just a matter of removing the table arg from
> > proc_sys_make_inode?
>
> Yes, for this error it's correct.
>
> There are also new implementors of the set_ownership callback in ipc/
> which need to be adapted.
>
> I'll send a new revision.
Thx. Not sure what your using as a base, but I suggest you use
linux-next so you get al the stuff going into the rc.
best
--
Joel Granados