[05/14] sysctl: Add a size arg to __register_sysctl_table
Commit Message
This is part of the effort to remove the sentinel element in the
ctl_table arrays. We add a table_size argument to
__register_sysctl_table and adjust callers, all of which pass ctl_table
pointers and need an explicit call to ARRAY_SIZE.
The new table_size argument does not yet have any effect in the
init_header call which is still dependent on the sentinel's presence.
table_size *does* however drive the `kzalloc` allocation in
__register_sysctl_table with no adverse effects as the allocated memory
is either one element greater than the calculated ctl_table array (for
the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
of the calculated ctl_table array (for the call from sysctl_net.c and
register_sysctl). This approach will allows us to "just" remove the
sentinel without further changes to __register_sysctl_table as
table_size will represent the exact size for all the callers at that
point.
Temporarily implement a size calculation in register_net_sysctl, which
is an indirection call for all the network register calls.
Signed-off-by: Joel Granados <j.granados@samsung.com>
---
fs/proc/proc_sysctl.c | 22 +++++++++++-----------
include/linux/sysctl.h | 2 +-
ipc/ipc_sysctl.c | 4 +++-
ipc/mq_sysctl.c | 4 +++-
kernel/ucount.c | 3 ++-
net/sysctl_net.c | 8 +++++++-
6 files changed, 27 insertions(+), 16 deletions(-)
Comments
On Wed, Jul 26, 2023 at 04:06:25PM +0200, Joel Granados wrote:
> This is part of the effort to remove the sentinel element in the
> ctl_table arrays. We add a table_size argument to
> __register_sysctl_table and adjust callers, all of which pass ctl_table
> pointers and need an explicit call to ARRAY_SIZE.
>
> The new table_size argument does not yet have any effect in the
> init_header call which is still dependent on the sentinel's presence.
> table_size *does* however drive the `kzalloc` allocation in
> __register_sysctl_table with no adverse effects as the allocated memory
> is either one element greater than the calculated ctl_table array (for
> the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
> of the calculated ctl_table array (for the call from sysctl_net.c and
> register_sysctl). This approach will allows us to "just" remove the
> sentinel without further changes to __register_sysctl_table as
> table_size will represent the exact size for all the callers at that
> point.
>
> Temporarily implement a size calculation in register_net_sysctl, which
> is an indirection call for all the network register calls.
>
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> ---
> fs/proc/proc_sysctl.c | 22 +++++++++++-----------
> include/linux/sysctl.h | 2 +-
> ipc/ipc_sysctl.c | 4 +++-
> ipc/mq_sysctl.c | 4 +++-
> kernel/ucount.c | 3 ++-
> net/sysctl_net.c | 8 +++++++-
> 6 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index fa1438f1a355..8d04f01a89c1 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
> */
> struct ctl_table_header *__register_sysctl_table(
> struct ctl_table_set *set,
> - const char *path, struct ctl_table *table)
> + const char *path, struct ctl_table *table, size_t table_size)
Hi Joel,
Please consider adding table_size to the kernel doc for this function.
...
On Fri, Jul 28, 2023 at 12:51:22PM +0200, Simon Horman wrote:
> On Wed, Jul 26, 2023 at 04:06:25PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel element in the
> > ctl_table arrays. We add a table_size argument to
> > __register_sysctl_table and adjust callers, all of which pass ctl_table
> > pointers and need an explicit call to ARRAY_SIZE.
> >
> > The new table_size argument does not yet have any effect in the
> > init_header call which is still dependent on the sentinel's presence.
> > table_size *does* however drive the `kzalloc` allocation in
> > __register_sysctl_table with no adverse effects as the allocated memory
> > is either one element greater than the calculated ctl_table array (for
> > the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
> > of the calculated ctl_table array (for the call from sysctl_net.c and
> > register_sysctl). This approach will allows us to "just" remove the
> > sentinel without further changes to __register_sysctl_table as
> > table_size will represent the exact size for all the callers at that
> > point.
> >
> > Temporarily implement a size calculation in register_net_sysctl, which
> > is an indirection call for all the network register calls.
> >
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > ---
> > fs/proc/proc_sysctl.c | 22 +++++++++++-----------
> > include/linux/sysctl.h | 2 +-
> > ipc/ipc_sysctl.c | 4 +++-
> > ipc/mq_sysctl.c | 4 +++-
> > kernel/ucount.c | 3 ++-
> > net/sysctl_net.c | 8 +++++++-
> > 6 files changed, 27 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > index fa1438f1a355..8d04f01a89c1 100644
> > --- a/fs/proc/proc_sysctl.c
> > +++ b/fs/proc/proc_sysctl.c
> > @@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
> > */
> > struct ctl_table_header *__register_sysctl_table(
> > struct ctl_table_set *set,
> > - const char *path, struct ctl_table *table)
> > + const char *path, struct ctl_table *table, size_t table_size)
>
> Hi Joel,
>
> Please consider adding table_size to the kernel doc for this function.
Good catch. Will do for V2.
>
> ...
@@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
*/
struct ctl_table_header *__register_sysctl_table(
struct ctl_table_set *set,
- const char *path, struct ctl_table *table)
+ const char *path, struct ctl_table *table, size_t table_size)
{
struct ctl_table_root *root = set->dir.header.root;
struct ctl_table_header *header;
- struct ctl_table_header h_tmp;
struct ctl_dir *dir;
- struct ctl_table *entry;
struct ctl_node *node;
- int nr_entries = 0;
-
- h_tmp.ctl_table = table;
- list_for_each_table_entry(entry, (&h_tmp))
- nr_entries++;
header = kzalloc(sizeof(struct ctl_table_header) +
- sizeof(struct ctl_node)*nr_entries, GFP_KERNEL_ACCOUNT);
+ sizeof(struct ctl_node)*table_size, GFP_KERNEL_ACCOUNT);
if (!header)
return NULL;
node = (struct ctl_node *)(header + 1);
- init_header(header, root, set, node, table, nr_entries);
+ init_header(header, root, set, node, table, table_size);
if (sysctl_check_table(path, header))
goto fail;
@@ -1423,8 +1416,15 @@ struct ctl_table_header *__register_sysctl_table(
*/
struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table)
{
+ int count = 0;
+ struct ctl_table *entry;
+ struct ctl_table_header t_hdr;
+
+ t_hdr.ctl_table = table;
+ list_for_each_table_entry(entry, (&t_hdr))
+ count++;
return __register_sysctl_table(&sysctl_table_root.default_set,
- path, table);
+ path, table, count);
}
EXPORT_SYMBOL(register_sysctl);
@@ -226,7 +226,7 @@ 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);
+ const char *path, struct ctl_table *table, size_t table_size);
struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table);
void unregister_sysctl_table(struct ctl_table_header * table);
@@ -259,7 +259,9 @@ bool setup_ipc_sysctls(struct ipc_namespace *ns)
tbl[i].data = NULL;
}
- ns->ipc_sysctls = __register_sysctl_table(&ns->ipc_set, "kernel", tbl);
+ ns->ipc_sysctls = __register_sysctl_table(&ns->ipc_set,
+ "kernel", tbl,
+ ARRAY_SIZE(ipc_sysctls));
}
if (!ns->ipc_sysctls) {
kfree(tbl);
@@ -109,7 +109,9 @@ bool setup_mq_sysctls(struct ipc_namespace *ns)
tbl[i].data = NULL;
}
- ns->mq_sysctls = __register_sysctl_table(&ns->mq_set, "fs/mqueue", tbl);
+ ns->mq_sysctls = __register_sysctl_table(&ns->mq_set,
+ "fs/mqueue", tbl,
+ ARRAY_SIZE(mq_sysctls));
}
if (!ns->mq_sysctls) {
kfree(tbl);
@@ -104,7 +104,8 @@ bool setup_userns_sysctls(struct user_namespace *ns)
for (i = 0; i < UCOUNT_COUNTS; i++) {
tbl[i].data = &ns->ucount_max[i];
}
- ns->sysctls = __register_sysctl_table(&ns->set, "user", tbl);
+ ns->sysctls = __register_sysctl_table(&ns->set, "user", tbl,
+ ARRAY_SIZE(user_table));
}
if (!ns->sysctls) {
kfree(tbl);
@@ -163,10 +163,16 @@ static void ensure_safe_net_sysctl(struct net *net, const char *path,
struct ctl_table_header *register_net_sysctl(struct net *net,
const char *path, struct ctl_table *table)
{
+ int count = 0;
+ struct ctl_table *entry;
+
if (!net_eq(net, &init_net))
ensure_safe_net_sysctl(net, path, table);
- return __register_sysctl_table(&net->sysctls, path, table);
+ for (entry = table; entry->procname; entry++)
+ count++;
+
+ return __register_sysctl_table(&net->sysctls, path, table, count);
}
EXPORT_SYMBOL_GPL(register_net_sysctl);