netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification
@ 2024-03-15 18:11 ` Thomas Weißschuh
  2024-03-15 18:11   ` [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table) Thomas Weißschuh
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-15 18:11 UTC (permalink / raw)
  To: Luis Chamberlain, Kees Cook, Joel Granados, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, linux-fsdevel, netdev, Thomas Weißschuh

The two patches were previously submitted on their own.
In commit f9436a5d0497
("sysctl: allow to change limits for posix messages queues")
a code dependency was introduced between the two callbacks.
This code dependency results in a dependency between the two patches, so
now they are submitted as a series.

The series is meant to be merged via the sysctl tree.

There is an upcoming series that will introduce a new implementation of
.set_ownership and .permissions which would need to be adapted [0].

These changes ere originally part of the sysctl-const series [1].
To slim down that series and reduce the message load on other
maintainers to a minimum, the patches are split out.

[0] https://lore.kernel.org/lkml/20240222160915.315255-1-aleksandr.mikhalitsyn@canonical.com/
[1] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-2-7a5060b11447@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v3:
- Drop now spurious argument in fs/proc/proc_sysctl.c
- Rebase on next-20240315
- Incorporate permissions patch.
- Link to v2 (ownership): https://lore.kernel.org/r/20240223-sysctl-const-ownership-v2-1-f9ba1795aaf2@weissschuh.net
- Link to v1 (permissions): https://lore.kernel.org/r/20231226-sysctl-const-permissions-v1-1-5cd3c91f6299@weissschuh.net

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/20231226-sysctl-const-ownership-v1-1-d78fdd744ba1@weissschuh.net

---
Thomas Weißschuh (2):
      sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)
      sysctl: treewide: constify argument ctl_table_root::permissions(table)

 fs/proc/proc_sysctl.c  | 2 +-
 include/linux/sysctl.h | 3 +--
 ipc/ipc_sysctl.c       | 5 ++---
 ipc/mq_sysctl.c        | 5 ++---
 kernel/ucount.c        | 2 +-
 net/sysctl_net.c       | 3 +--
 6 files changed, 8 insertions(+), 12 deletions(-)
---
base-commit: a1e7655b77e3391b58ac28256789ea45b1685abb
change-id: 20231226-sysctl-const-ownership-ff75e67b4eea

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)
  2024-03-15 18:11 ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Thomas Weißschuh
@ 2024-03-15 18:11   ` Thomas Weißschuh
  2024-03-22 12:45     ` Joel Granados
  2024-03-15 18:11   ` [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table) Thomas Weißschuh
  2024-03-22 12:47   ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Joel Granados
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-15 18:11 UTC (permalink / raw)
  To: Luis Chamberlain, Kees Cook, Joel Granados, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, linux-fsdevel, netdev, Thomas Weißschuh

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 <linux@weissschuh.net>
---
 fs/proc/proc_sysctl.c  | 2 +-
 include/linux/sysctl.h | 1 -
 ipc/ipc_sysctl.c       | 3 +--
 ipc/mq_sysctl.c        | 3 +--
 net/sysctl_net.c       | 1 -
 5 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 37cde0efee57..ed3a41ed9705 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -480,7 +480,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
 	}
 
 	if (root->set_ownership)
-		root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
+		root->set_ownership(head, &inode->i_uid, &inode->i_gid);
 	else {
 		inode->i_uid = GLOBAL_ROOT_UID;
 		inode->i_gid = GLOBAL_ROOT_GID;
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/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 45cb1dabce29..1a5085e5b178 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -192,7 +192,6 @@ static int set_is_seen(struct ctl_table_set *set)
 }
 
 static void ipc_set_ownership(struct ctl_table_header *head,
-			      struct ctl_table *table,
 			      kuid_t *uid, kgid_t *gid)
 {
 	struct ipc_namespace *ns =
@@ -224,7 +223,7 @@ static int ipc_permissions(struct ctl_table_header *head, struct ctl_table *tabl
 		kuid_t ns_root_uid;
 		kgid_t ns_root_gid;
 
-		ipc_set_ownership(head, table, &ns_root_uid, &ns_root_gid);
+		ipc_set_ownership(head, &ns_root_uid, &ns_root_gid);
 
 		if (uid_eq(current_euid(), ns_root_uid))
 			mode >>= 6;
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index 21fba3a6edaf..6bb1c5397c69 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -78,7 +78,6 @@ static int set_is_seen(struct ctl_table_set *set)
 }
 
 static void mq_set_ownership(struct ctl_table_header *head,
-			     struct ctl_table *table,
 			     kuid_t *uid, kgid_t *gid)
 {
 	struct ipc_namespace *ns =
@@ -97,7 +96,7 @@ static int mq_permissions(struct ctl_table_header *head, struct ctl_table *table
 	kuid_t ns_root_uid;
 	kgid_t ns_root_gid;
 
-	mq_set_ownership(head, table, &ns_root_uid, &ns_root_gid);
+	mq_set_ownership(head, &ns_root_uid, &ns_root_gid);
 
 	if (uid_eq(current_euid(), ns_root_uid))
 		mode >>= 6;
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);

-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table)
  2024-03-15 18:11 ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Thomas Weißschuh
  2024-03-15 18:11   ` [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table) Thomas Weißschuh
@ 2024-03-15 18:11   ` Thomas Weißschuh
  2024-03-22 12:32     ` Joel Granados
  2024-03-22 12:47   ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Joel Granados
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-15 18:11 UTC (permalink / raw)
  To: Luis Chamberlain, Kees Cook, Joel Granados, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, linux-fsdevel, netdev, Thomas Weißschuh

The permissions callback is not supposed to modify the ctl_table.
Enforce this expectation via the typesystem.

The patch was created with the following coccinelle script:

  @@
  identifier func, head, ctl;
  @@

  int func(
    struct ctl_table_header *head,
  - struct ctl_table *ctl)
  + const struct ctl_table *ctl)
  { ... }

(insert_entry() from fs/proc/proc_sysctl.c is a false-positive)

The three changed locations were validated through manually inspection
and compilation.

In addition a search for '.permissions =' was done over the full tree to
look for places that were missed by coccinelle.
None were found.

This change also is a step to put "struct ctl_table" into .rodata
throughout the kernel.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/linux/sysctl.h | 2 +-
 ipc/ipc_sysctl.c       | 2 +-
 ipc/mq_sysctl.c        | 2 +-
 kernel/ucount.c        | 2 +-
 net/sysctl_net.c       | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 60333a6b9370..f9214de0490c 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -206,7 +206,7 @@ struct ctl_table_root {
 	struct ctl_table_set *(*lookup)(struct ctl_table_root *root);
 	void (*set_ownership)(struct ctl_table_header *head,
 			      kuid_t *uid, kgid_t *gid);
-	int (*permissions)(struct ctl_table_header *head, struct ctl_table *table);
+	int (*permissions)(struct ctl_table_header *head, const struct ctl_table *table);
 };
 
 #define register_sysctl(path, table)	\
diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 1a5085e5b178..19b2a67aef40 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -204,7 +204,7 @@ static void ipc_set_ownership(struct ctl_table_header *head,
 	*gid = gid_valid(ns_root_gid) ? ns_root_gid : GLOBAL_ROOT_GID;
 }
 
-static int ipc_permissions(struct ctl_table_header *head, struct ctl_table *table)
+static int ipc_permissions(struct ctl_table_header *head, const struct ctl_table *table)
 {
 	int mode = table->mode;
 
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index 6bb1c5397c69..43c0825da9e8 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -90,7 +90,7 @@ static void mq_set_ownership(struct ctl_table_header *head,
 	*gid = gid_valid(ns_root_gid) ? ns_root_gid : GLOBAL_ROOT_GID;
 }
 
-static int mq_permissions(struct ctl_table_header *head, struct ctl_table *table)
+static int mq_permissions(struct ctl_table_header *head, const struct ctl_table *table)
 {
 	int mode = table->mode;
 	kuid_t ns_root_uid;
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 4aa6166cb856..90300840256b 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -38,7 +38,7 @@ static int set_is_seen(struct ctl_table_set *set)
 }
 
 static int set_permissions(struct ctl_table_header *head,
-				  struct ctl_table *table)
+			   const struct ctl_table *table)
 {
 	struct user_namespace *user_ns =
 		container_of(head->set, struct user_namespace, set);
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index a0a7a79991f9..f5017012a049 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -40,7 +40,7 @@ static int is_seen(struct ctl_table_set *set)
 
 /* Return standard mode bits for table entry. */
 static int net_ctl_permissions(struct ctl_table_header *head,
-			       struct ctl_table *table)
+			       const struct ctl_table *table)
 {
 	struct net *net = container_of(head->set, struct net, sysctls);
 

-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table)
  2024-03-15 18:11   ` [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table) Thomas Weißschuh
@ 2024-03-22 12:32     ` Joel Granados
  0 siblings, 0 replies; 7+ messages in thread
From: Joel Granados @ 2024-03-22 12:32 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Luis Chamberlain, Kees Cook, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, linux-fsdevel, netdev

[-- Attachment #1: Type: text/plain, Size: 4901 bytes --]

On Fri, Mar 15, 2024 at 07:11:31PM +0100, Thomas Weißschuh wrote:
> The permissions callback is not supposed to modify the ctl_table.
> Enforce this expectation via the typesystem.
> 
> The patch was created with the following coccinelle script:
> 
>   @@
>   identifier func, head, ctl;
>   @@
> 
>   int func(
>     struct ctl_table_header *head,
>   - struct ctl_table *ctl)
>   + const struct ctl_table *ctl)
>   { ... }
> 
> (insert_entry() from fs/proc/proc_sysctl.c is a false-positive)
> 
> The three changed locations were validated through manually inspection
> and compilation.
Will remove this when I add it to constfy branch as it is unclear (for
me) what "manually inspection" is and also I do not know what config you
used to compile. IMO, we can just do without it.

> 
> In addition a search for '.permissions =' was done over the full tree to
> look for places that were missed by coccinelle.
> None were found.
> 
> This change also is a step to put "struct ctl_table" into .rodata
> throughout the kernel.

This LGTM. Will add this to the constfy testing branch with these
changes in the commit message:
"""
sysctl: treewide: constify argument ctl_table_root::permissions(table)

The permissions callback should not modify the ctl_table. Enforce this
expectation via the typesystem. This is a step to put "struct ctl_table"
into .rodata throughout the kernel.

The patch was created with the following coccinelle script:

  @@
  identifier func, head, ctl;
  @@

  int func(
    struct ctl_table_header *head,
  - struct ctl_table *ctl)
  + const struct ctl_table *ctl)
  { ... }

(insert_entry() from fs/proc/proc_sysctl.c is a false-positive)

No additional occurances of '.permissions =' were found after a
tree-wide search for places missed by the conccinelle script.

"""

Reviewed-by: Joel Granados <j.granados@samsung.com>
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  include/linux/sysctl.h | 2 +-
>  ipc/ipc_sysctl.c       | 2 +-
>  ipc/mq_sysctl.c        | 2 +-
>  kernel/ucount.c        | 2 +-
>  net/sysctl_net.c       | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 60333a6b9370..f9214de0490c 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -206,7 +206,7 @@ struct ctl_table_root {
>  	struct ctl_table_set *(*lookup)(struct ctl_table_root *root);
>  	void (*set_ownership)(struct ctl_table_header *head,
>  			      kuid_t *uid, kgid_t *gid);
> -	int (*permissions)(struct ctl_table_header *head, struct ctl_table *table);
> +	int (*permissions)(struct ctl_table_header *head, const struct ctl_table *table);
>  };
>  
>  #define register_sysctl(path, table)	\
> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> index 1a5085e5b178..19b2a67aef40 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -204,7 +204,7 @@ static void ipc_set_ownership(struct ctl_table_header *head,
>  	*gid = gid_valid(ns_root_gid) ? ns_root_gid : GLOBAL_ROOT_GID;
>  }
>  
> -static int ipc_permissions(struct ctl_table_header *head, struct ctl_table *table)
> +static int ipc_permissions(struct ctl_table_header *head, const struct ctl_table *table)
>  {
>  	int mode = table->mode;
>  
> diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
> index 6bb1c5397c69..43c0825da9e8 100644
> --- a/ipc/mq_sysctl.c
> +++ b/ipc/mq_sysctl.c
> @@ -90,7 +90,7 @@ static void mq_set_ownership(struct ctl_table_header *head,
>  	*gid = gid_valid(ns_root_gid) ? ns_root_gid : GLOBAL_ROOT_GID;
>  }
>  
> -static int mq_permissions(struct ctl_table_header *head, struct ctl_table *table)
> +static int mq_permissions(struct ctl_table_header *head, const struct ctl_table *table)
>  {
>  	int mode = table->mode;
>  	kuid_t ns_root_uid;
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 4aa6166cb856..90300840256b 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -38,7 +38,7 @@ static int set_is_seen(struct ctl_table_set *set)
>  }
>  
>  static int set_permissions(struct ctl_table_header *head,
> -				  struct ctl_table *table)
> +			   const struct ctl_table *table)
>  {
>  	struct user_namespace *user_ns =
>  		container_of(head->set, struct user_namespace, set);
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index a0a7a79991f9..f5017012a049 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -40,7 +40,7 @@ static int is_seen(struct ctl_table_set *set)
>  
>  /* Return standard mode bits for table entry. */
>  static int net_ctl_permissions(struct ctl_table_header *head,
> -			       struct ctl_table *table)
> +			       const struct ctl_table *table)
>  {
>  	struct net *net = container_of(head->set, struct net, sysctls);
>  
> 
> -- 
> 2.44.0
> 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)
  2024-03-15 18:11   ` [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table) Thomas Weißschuh
@ 2024-03-22 12:45     ` Joel Granados
  0 siblings, 0 replies; 7+ messages in thread
From: Joel Granados @ 2024-03-22 12:45 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Luis Chamberlain, Kees Cook, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, linux-fsdevel, netdev

[-- Attachment #1: Type: text/plain, Size: 5011 bytes --]

On Fri, Mar 15, 2024 at 07:11:30PM +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.
Will drop this from the commit message when I add it to constfy branch.
For the same reasons as before.

here is the commit message that I'll use
"""
sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)

Remove the 'table' argument from set_ownership as it is never used. This
change is a step towards putting "struct ctl_table" into .rodata and
eventually having sysctl core only use "const struct ctl_table".

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)
  { ... }

No additional occurrences of 'set_ownership' were found after doing a
tree-wide search.

"""

Reviewed-by: Joel Granados <j.granados@samsung.com>

thx.
> 
> 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 <linux@weissschuh.net>
> ---
>  fs/proc/proc_sysctl.c  | 2 +-
>  include/linux/sysctl.h | 1 -
>  ipc/ipc_sysctl.c       | 3 +--
>  ipc/mq_sysctl.c        | 3 +--
>  net/sysctl_net.c       | 1 -
>  5 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 37cde0efee57..ed3a41ed9705 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -480,7 +480,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
>  	}
>  
>  	if (root->set_ownership)
> -		root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> +		root->set_ownership(head, &inode->i_uid, &inode->i_gid);
>  	else {
>  		inode->i_uid = GLOBAL_ROOT_UID;
>  		inode->i_gid = GLOBAL_ROOT_GID;
> 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/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> index 45cb1dabce29..1a5085e5b178 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -192,7 +192,6 @@ static int set_is_seen(struct ctl_table_set *set)
>  }
>  
>  static void ipc_set_ownership(struct ctl_table_header *head,
> -			      struct ctl_table *table,
>  			      kuid_t *uid, kgid_t *gid)
>  {
>  	struct ipc_namespace *ns =
> @@ -224,7 +223,7 @@ static int ipc_permissions(struct ctl_table_header *head, struct ctl_table *tabl
>  		kuid_t ns_root_uid;
>  		kgid_t ns_root_gid;
>  
> -		ipc_set_ownership(head, table, &ns_root_uid, &ns_root_gid);
> +		ipc_set_ownership(head, &ns_root_uid, &ns_root_gid);
>  
>  		if (uid_eq(current_euid(), ns_root_uid))
>  			mode >>= 6;
> diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
> index 21fba3a6edaf..6bb1c5397c69 100644
> --- a/ipc/mq_sysctl.c
> +++ b/ipc/mq_sysctl.c
> @@ -78,7 +78,6 @@ static int set_is_seen(struct ctl_table_set *set)
>  }
>  
>  static void mq_set_ownership(struct ctl_table_header *head,
> -			     struct ctl_table *table,
>  			     kuid_t *uid, kgid_t *gid)
>  {
>  	struct ipc_namespace *ns =
> @@ -97,7 +96,7 @@ static int mq_permissions(struct ctl_table_header *head, struct ctl_table *table
>  	kuid_t ns_root_uid;
>  	kgid_t ns_root_gid;
>  
> -	mq_set_ownership(head, table, &ns_root_uid, &ns_root_gid);
> +	mq_set_ownership(head, &ns_root_uid, &ns_root_gid);
>  
>  	if (uid_eq(current_euid(), ns_root_uid))
>  		mode >>= 6;
> 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);
> 
> -- 
> 2.44.0
> 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification
  2024-03-15 18:11 ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Thomas Weißschuh
  2024-03-15 18:11   ` [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table) Thomas Weißschuh
  2024-03-15 18:11   ` [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table) Thomas Weißschuh
@ 2024-03-22 12:47   ` Joel Granados
  2024-03-22 16:31     ` Thomas Weißschuh
  2 siblings, 1 reply; 7+ messages in thread
From: Joel Granados @ 2024-03-22 12:47 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Luis Chamberlain, Kees Cook, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, linux-fsdevel, netdev

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

On Fri, Mar 15, 2024 at 07:11:29PM +0100, Thomas Weißschuh wrote:
> The two patches were previously submitted on their own.
> In commit f9436a5d0497
> ("sysctl: allow to change limits for posix messages queues")
> a code dependency was introduced between the two callbacks.
> This code dependency results in a dependency between the two patches, so
> now they are submitted as a series.
> 
> The series is meant to be merged via the sysctl tree.
> 
> There is an upcoming series that will introduce a new implementation of
> .set_ownership and .permissions which would need to be adapted [0].
> 
> These changes ere originally part of the sysctl-const series [1].
> To slim down that series and reduce the message load on other
> maintainers to a minimum, the patches are split out.
> 
> [0] https://lore.kernel.org/lkml/20240222160915.315255-1-aleksandr.mikhalitsyn@canonical.com/
> [1] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-2-7a5060b11447@weissschuh.net/
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Changes in v3:
> - Drop now spurious argument in fs/proc/proc_sysctl.c
> - Rebase on next-20240315
> - Incorporate permissions patch.
> - Link to v2 (ownership): https://lore.kernel.org/r/20240223-sysctl-const-ownership-v2-1-f9ba1795aaf2@weissschuh.net
> - Link to v1 (permissions): https://lore.kernel.org/r/20231226-sysctl-const-permissions-v1-1-5cd3c91f6299@weissschuh.net
> 
> 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/20231226-sysctl-const-ownership-v1-1-d78fdd744ba1@weissschuh.net
> 
> ---
> Thomas Weißschuh (2):
>       sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)
>       sysctl: treewide: constify argument ctl_table_root::permissions(table)
> 
>  fs/proc/proc_sysctl.c  | 2 +-
>  include/linux/sysctl.h | 3 +--
>  ipc/ipc_sysctl.c       | 5 ++---
>  ipc/mq_sysctl.c        | 5 ++---
>  kernel/ucount.c        | 2 +-
>  net/sysctl_net.c       | 3 +--
>  6 files changed, 8 insertions(+), 12 deletions(-)
> ---
> base-commit: a1e7655b77e3391b58ac28256789ea45b1685abb
> change-id: 20231226-sysctl-const-ownership-ff75e67b4eea
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 

Will put this to test and then try to rebase it to 6.9-rc1 once it comes
out.

Thx.

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification
  2024-03-22 12:47   ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Joel Granados
@ 2024-03-22 16:31     ` Thomas Weißschuh
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2024-03-22 16:31 UTC (permalink / raw)
  To: Joel Granados
  Cc: Luis Chamberlain, Kees Cook, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, linux-fsdevel, netdev

On 2024-03-22 13:47:09+0100, Joel Granados wrote:
> On Fri, Mar 15, 2024 at 07:11:29PM +0100, Thomas Weißschuh wrote:
> > The two patches were previously submitted on their own.
> > In commit f9436a5d0497
> > ("sysctl: allow to change limits for posix messages queues")
> > a code dependency was introduced between the two callbacks.
> > This code dependency results in a dependency between the two patches, so
> > now they are submitted as a series.
> > 
> > The series is meant to be merged via the sysctl tree.
> > 
> > There is an upcoming series that will introduce a new implementation of
> > .set_ownership and .permissions which would need to be adapted [0].
> > 
> > These changes ere originally part of the sysctl-const series [1].
> > To slim down that series and reduce the message load on other
> > maintainers to a minimum, the patches are split out.
> > 
> > [0] https://lore.kernel.org/lkml/20240222160915.315255-1-aleksandr.mikhalitsyn@canonical.com/
> > [1] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-2-7a5060b11447@weissschuh.net/
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Changes in v3:
> > - Drop now spurious argument in fs/proc/proc_sysctl.c
> > - Rebase on next-20240315
> > - Incorporate permissions patch.
> > - Link to v2 (ownership): https://lore.kernel.org/r/20240223-sysctl-const-ownership-v2-1-f9ba1795aaf2@weissschuh.net
> > - Link to v1 (permissions): https://lore.kernel.org/r/20231226-sysctl-const-permissions-v1-1-5cd3c91f6299@weissschuh.net
> > 
> > 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/20231226-sysctl-const-ownership-v1-1-d78fdd744ba1@weissschuh.net
> > 
> > ---
> > Thomas Weißschuh (2):
> >       sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table)
> >       sysctl: treewide: constify argument ctl_table_root::permissions(table)
> > 
> >  fs/proc/proc_sysctl.c  | 2 +-
> >  include/linux/sysctl.h | 3 +--
> >  ipc/ipc_sysctl.c       | 5 ++---
> >  ipc/mq_sysctl.c        | 5 ++---
> >  kernel/ucount.c        | 2 +-
> >  net/sysctl_net.c       | 3 +--
> >  6 files changed, 8 insertions(+), 12 deletions(-)
> > ---
> > base-commit: a1e7655b77e3391b58ac28256789ea45b1685abb
> > change-id: 20231226-sysctl-const-ownership-ff75e67b4eea
> > 
> > Best regards,
> > -- 
> > Thomas Weißschuh <linux@weissschuh.net>
> > 
> 
> Will put this to test and then try to rebase it to 6.9-rc1 once it comes
> out.

Thanks!
Your changes to the commit messages look good.

For my other changes I'm planning to resubmit all of them during the
weekend or next week.


Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-03-22 16:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20240315181141eucas1p267385cd08f77d720e58b038be06d292e@eucas1p2.samsung.com>
2024-03-15 18:11 ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Thomas Weißschuh
2024-03-15 18:11   ` [PATCH v3 1/2] sysctl: treewide: drop unused argument ctl_table_root::set_ownership(table) Thomas Weißschuh
2024-03-22 12:45     ` Joel Granados
2024-03-15 18:11   ` [PATCH v3 2/2] sysctl: treewide: constify argument ctl_table_root::permissions(table) Thomas Weißschuh
2024-03-22 12:32     ` Joel Granados
2024-03-22 12:47   ` [PATCH v3 0/2] sysctl: treewide: prepare ctl_table_root for ctl_table constification Joel Granados
2024-03-22 16:31     ` Thomas Weißschuh

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).