From 93810936a6bfbc538771914836fca24570e3bcc5 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 23 May 2023 14:22:14 +0200 Subject: parport: Remove register_sysctl_table from parport_proc_register This is part of the general push to deprecate register_sysctl_paths and register_sysctl_table. Register dev/parport/PORTNAME and dev/parport/PORTNAME/devices. Temporary allocation for name is freed at the end of the function. Remove all the struct elements that are no longer used in the parport_device_sysctl_template struct. Add parport specific defines that hide the base path sizes. To make sure the resulting directory structure did not change we made sure that `find /proc/sys/dev/ | sha1sum` was the same before and after the change. Signed-off-by: Joel Granados Reported-by: kernel test robot Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/ Reported-by: Dan Carpenter Reviewed-by: Luis Chamberlain Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 91 +++++++++++++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 31 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index d740eba3c099..28a37e0ef98c 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -32,6 +32,13 @@ #define PARPORT_MAX_TIMESLICE_VALUE ((unsigned long) HZ) #define PARPORT_MIN_SPINTIME_VALUE 1 #define PARPORT_MAX_SPINTIME_VALUE 1000 +/* + * PARPORT_BASE_* is the size of the known parts of the sysctl path + * in dev/partport/%s/devices/%s. "dev/parport/"(12), "/devices/"(9 + * and null char(1). + */ +#define PARPORT_BASE_PATH_SIZE 13 +#define PARPORT_BASE_DEVICES_PATH_SIZE 22 static int do_active_device(struct ctl_table *table, int write, void *result, size_t *lenp, loff_t *ppos) @@ -260,9 +267,6 @@ struct parport_sysctl_table { struct ctl_table_header *sysctl_header; struct ctl_table vars[12]; struct ctl_table device_dir[2]; - struct ctl_table port_dir[2]; - struct ctl_table parport_dir[2]; - struct ctl_table dev_dir[2]; }; static const struct parport_sysctl_table parport_sysctl_template = { @@ -305,7 +309,6 @@ static const struct parport_sysctl_table parport_sysctl_template = { .mode = 0444, .proc_handler = do_hardware_modes }, - PARPORT_DEVICES_ROOT_DIR, #ifdef CONFIG_PARPORT_1284 { .procname = "autoprobe", @@ -355,18 +358,6 @@ static const struct parport_sysctl_table parport_sysctl_template = { }, {} }, - { - PARPORT_PORT_DIR(NULL), - {} - }, - { - PARPORT_PARPORT_DIR(NULL), - {} - }, - { - PARPORT_DEV_DIR(NULL), - {} - } }; struct parport_device_sysctl_table @@ -473,11 +464,13 @@ parport_default_sysctl_table = { } }; - int parport_proc_register(struct parport *port) { struct parport_sysctl_table *t; - int i; + struct ctl_table_header *devices_h; + char *tmp_dir_path; + size_t tmp_path_len, port_name_len; + int bytes_written, i, err = 0; t = kmemdup(&parport_sysctl_template, sizeof(*t), GFP_KERNEL); if (t == NULL) @@ -485,28 +478,64 @@ int parport_proc_register(struct parport *port) t->device_dir[0].extra1 = port; - for (i = 0; i < 5; i++) + t->vars[0].data = &port->spintime; + for (i = 0; i < 5; i++) { t->vars[i].extra1 = port; + t->vars[5 + i].extra2 = &port->probe_info[i]; + } - t->vars[0].data = &port->spintime; - t->vars[5].child = t->device_dir; - - for (i = 0; i < 5; i++) - t->vars[6 + i].extra2 = &port->probe_info[i]; + port_name_len = strnlen(port->name, PARPORT_NAME_MAX_LEN); + /* + * Allocate a buffer for two paths: dev/parport/PORT and dev/parport/PORT/devices. + * We calculate for the second as that will give us enough for the first. + */ + tmp_path_len = PARPORT_BASE_DEVICES_PATH_SIZE + port_name_len; + tmp_dir_path = kzalloc(tmp_path_len, GFP_KERNEL); + if (!tmp_dir_path) { + err = -ENOMEM; + goto exit_free_t; + } - t->port_dir[0].procname = port->name; + bytes_written = snprintf(tmp_dir_path, tmp_path_len, + "dev/parport/%s/devices", port->name); + if (tmp_path_len <= bytes_written) { + err = -ENOENT; + goto exit_free_tmp_dir_path; + } + devices_h = register_sysctl(tmp_dir_path, t->device_dir); + if (devices_h == NULL) { + err = -ENOENT; + goto exit_free_tmp_dir_path; + } - t->port_dir[0].child = t->vars; - t->parport_dir[0].child = t->port_dir; - t->dev_dir[0].child = t->parport_dir; + tmp_path_len = PARPORT_BASE_PATH_SIZE + port_name_len; + bytes_written = snprintf(tmp_dir_path, tmp_path_len, + "dev/parport/%s", port->name); + if (tmp_path_len <= bytes_written) { + err = -ENOENT; + goto unregister_devices_h; + } - t->sysctl_header = register_sysctl_table(t->dev_dir); + t->sysctl_header = register_sysctl(tmp_dir_path, t->vars); if (t->sysctl_header == NULL) { - kfree(t); - t = NULL; + err = -ENOENT; + goto unregister_devices_h; } + port->sysctl_table = t; + + kfree(tmp_dir_path); return 0; + +unregister_devices_h: + unregister_sysctl_table(devices_h); + +exit_free_tmp_dir_path: + kfree(tmp_dir_path); + +exit_free_t: + kfree(t); + return err; } int parport_proc_unregister(struct parport *port) -- cgit v1.2.3 From 4199a64a1c13c58a48917d1eb6492c32b4496bd9 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 23 May 2023 14:22:15 +0200 Subject: parport: Remove register_sysctl_table from parport_device_proc_register This is part of the general push to deprecate register_sysctl_paths and register_sysctl_table. We use a temp allocation to include both port and device name in proc. Allocated mem is freed at the end. The unused parport_device_sysctl_template struct elements that are not used are removed. Signed-off-by: Joel Granados Reported-by: kernel test robot Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/ Reported-by: Dan Carpenter Reviewed-by: Luis Chamberlain Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 56 +++++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 24 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 28a37e0ef98c..22d211c95168 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -384,6 +384,7 @@ parport_device_sysctl_template = { .extra1 = (void*) &parport_min_timeslice_value, .extra2 = (void*) &parport_max_timeslice_value }, + {} }, { { @@ -394,22 +395,6 @@ parport_device_sysctl_template = { .child = NULL }, {} - }, - { - PARPORT_DEVICES_ROOT_DIR, - {} - }, - { - PARPORT_PORT_DIR(NULL), - {} - }, - { - PARPORT_PARPORT_DIR(NULL), - {} - }, - { - PARPORT_DEV_DIR(NULL), - {} } }; @@ -551,30 +536,53 @@ int parport_proc_unregister(struct parport *port) int parport_device_proc_register(struct pardevice *device) { + int bytes_written, err = 0; struct parport_device_sysctl_table *t; struct parport * port = device->port; + size_t port_name_len, device_name_len, tmp_dir_path_len; + char *tmp_dir_path; t = kmemdup(&parport_device_sysctl_template, sizeof(*t), GFP_KERNEL); if (t == NULL) return -ENOMEM; - t->dev_dir[0].child = t->parport_dir; - t->parport_dir[0].child = t->port_dir; - t->port_dir[0].procname = port->name; - t->port_dir[0].child = t->devices_root_dir; - t->devices_root_dir[0].child = t->device_dir; + port_name_len = strnlen(port->name, PARPORT_NAME_MAX_LEN); + device_name_len = strnlen(device->name, PATH_MAX); + + /* Allocate a buffer for two paths: dev/parport/PORT/devices/DEVICE. */ + tmp_dir_path_len = PARPORT_BASE_DEVICES_PATH_SIZE + port_name_len + device_name_len; + tmp_dir_path = kzalloc(tmp_dir_path_len, GFP_KERNEL); + if (!tmp_dir_path) { + err = -ENOMEM; + goto exit_free_t; + } + + bytes_written = snprintf(tmp_dir_path, tmp_dir_path_len, "dev/parport/%s/devices/%s", + port->name, device->name); + if (tmp_dir_path_len <= bytes_written) { + err = -ENOENT; + goto exit_free_path; + } - t->device_dir[0].procname = device->name; - t->device_dir[0].child = t->vars; t->vars[0].data = &device->timeslice; - t->sysctl_header = register_sysctl_table(t->dev_dir); + t->sysctl_header = register_sysctl(tmp_dir_path, t->vars); if (t->sysctl_header == NULL) { kfree(t); t = NULL; } device->sysctl_table = t; + + kfree(tmp_dir_path); return 0; + +exit_free_path: + kfree(tmp_dir_path); + +exit_free_t: + kfree(t); + + return err; } int parport_device_proc_unregister(struct pardevice *device) -- cgit v1.2.3 From 02ea13480f3ffea36a1f9e549b503402f0b299ca Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 23 May 2023 14:22:16 +0200 Subject: parport: Remove register_sysctl_table from parport_default_proc_register This is part of the general push to deprecate register_sysctl_paths and register_sysctl_table. Simply change the full path "dev/parport/default" to point to an already existing set of table entries (vars). We also remove the unused elements from parport_default_table. To make sure the resulting directory structure did not change we made sure that `find /proc/sys/dev/ | sha1sum` was the same before and after the change. Signed-off-by: Joel Granados Reviewed-by: Luis Chamberlain Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 22d211c95168..1a26918d2cc8 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -430,22 +430,6 @@ parport_default_sysctl_table = { .extra2 = (void*) &parport_max_spintime_value }, {} - }, - { - { - .procname = "default", - .mode = 0555, - .child = parport_default_sysctl_table.vars - }, - {} - }, - { - PARPORT_PARPORT_DIR(parport_default_sysctl_table.default_dir), - {} - }, - { - PARPORT_DEV_DIR(parport_default_sysctl_table.parport_dir), - {} } }; @@ -601,7 +585,7 @@ static int __init parport_default_proc_register(void) int ret; parport_default_sysctl_table.sysctl_header = - register_sysctl_table(parport_default_sysctl_table.dev_dir); + register_sysctl("dev/parport/default", parport_default_sysctl_table.vars); if (!parport_default_sysctl_table.sysctl_header) return -ENOMEM; ret = parport_bus_init(); -- cgit v1.2.3 From 9ad0a4e7c2dd101be78e8621c204cec742ee1ce7 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Tue, 23 May 2023 14:22:17 +0200 Subject: parport: Removed sysctl related defines The partport driver used to rely on defines to include different directories in sysctl. Now that we have made the transition to register_sysctl from regsiter_sysctl_table, they are no longer needed. Signed-off-by: Joel Granados Reviewed-by: Luis Chamberlain Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 1a26918d2cc8..cbb1fb5127ce 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -243,13 +243,6 @@ do { \ return 0; } -#define PARPORT_PORT_DIR(CHILD) { .procname = NULL, .mode = 0555, .child = CHILD } -#define PARPORT_PARPORT_DIR(CHILD) { .procname = "parport", \ - .mode = 0555, .child = CHILD } -#define PARPORT_DEV_DIR(CHILD) { .procname = "dev", .mode = 0555, .child = CHILD } -#define PARPORT_DEVICES_ROOT_DIR { .procname = "devices", \ - .mode = 0555, .child = NULL } - static const unsigned long parport_min_timeslice_value = PARPORT_MIN_TIMESLICE_VALUE; -- cgit v1.2.3 From 37e9981e33e4d308c323a56bb908aa54c8f041a8 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Fri, 16 Jun 2023 10:59:15 +0200 Subject: parport: plug a sysctl register leak parport registers two sysctl directories in the parport_proc_register function but only one of them was getting unregistered in parport_proc_unregister. Keep track of both sysctl table headers and handle them together when (un)registering. Signed-off-by: Joel Granados Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index cbb1fb5127ce..0f2d2e1ee28e 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -257,14 +257,16 @@ PARPORT_MAX_SPINTIME_VALUE; struct parport_sysctl_table { - struct ctl_table_header *sysctl_header; + struct ctl_table_header *port_header; + struct ctl_table_header *devices_header; struct ctl_table vars[12]; struct ctl_table device_dir[2]; }; static const struct parport_sysctl_table parport_sysctl_template = { - .sysctl_header = NULL, - { + .port_header = NULL, + .devices_header = NULL, + { { .procname = "spintime", .data = NULL, @@ -429,7 +431,6 @@ parport_default_sysctl_table = { int parport_proc_register(struct parport *port) { struct parport_sysctl_table *t; - struct ctl_table_header *devices_h; char *tmp_dir_path; size_t tmp_path_len, port_name_len; int bytes_written, i, err = 0; @@ -464,8 +465,8 @@ int parport_proc_register(struct parport *port) err = -ENOENT; goto exit_free_tmp_dir_path; } - devices_h = register_sysctl(tmp_dir_path, t->device_dir); - if (devices_h == NULL) { + t->devices_header = register_sysctl(tmp_dir_path, t->device_dir); + if (t->devices_header == NULL) { err = -ENOENT; goto exit_free_tmp_dir_path; } @@ -478,8 +479,8 @@ int parport_proc_register(struct parport *port) goto unregister_devices_h; } - t->sysctl_header = register_sysctl(tmp_dir_path, t->vars); - if (t->sysctl_header == NULL) { + t->port_header = register_sysctl(tmp_dir_path, t->vars); + if (t->port_header == NULL) { err = -ENOENT; goto unregister_devices_h; } @@ -490,7 +491,7 @@ int parport_proc_register(struct parport *port) return 0; unregister_devices_h: - unregister_sysctl_table(devices_h); + unregister_sysctl_table(t->devices_header); exit_free_tmp_dir_path: kfree(tmp_dir_path); @@ -505,7 +506,8 @@ int parport_proc_unregister(struct parport *port) if (port->sysctl_table) { struct parport_sysctl_table *t = port->sysctl_table; port->sysctl_table = NULL; - unregister_sysctl_table(t->sysctl_header); + unregister_sysctl_table(t->devices_header); + unregister_sysctl_table(t->port_header); kfree(t); } return 0; -- cgit v1.2.3 From 2f2665c13af4895b26761107c2f637c2f112d8e9 Mon Sep 17 00:00:00 2001 From: Joel Granados Date: Fri, 16 Jun 2023 10:59:22 +0200 Subject: sysctl: replace child with an enumeration This is part of the effort to remove the empty element at the end of ctl_table structs. "child" was a deprecated elem in this struct and was being used to differentiate between two types of ctl_tables: "normal" and "permanently emtpy". What changed?: * Replace "child" with an enumeration that will have two values: the default (0) and the permanently empty (1). The latter is left at zero so when struct ctl_table is created with kzalloc or in a local context, it will have the zero value by default. We document the new enum with kdoc. * Remove the "empty child" check from sysctl_check_table * Remove count_subheaders function as there is no longer a need to calculate how many headers there are for every child * Remove the recursive call to unregister_sysctl_table as there is no need to traverse down the child tree any longer * Add a new SYSCTL_PERM_EMPTY_DIR binary flag * Remove the last remanence of child from partport/procfs.c Signed-off-by: Joel Granados Signed-off-by: Luis Chamberlain --- drivers/parport/procfs.c | 1 - fs/proc/proc_sysctl.c | 81 +++++++++++------------------------------------- include/linux/sysctl.h | 14 +++++++-- 3 files changed, 30 insertions(+), 66 deletions(-) (limited to 'drivers/parport/procfs.c') diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index 0f2d2e1ee28e..4e5b972c3e26 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -387,7 +387,6 @@ parport_device_sysctl_template = { .data = NULL, .maxlen = 0, .mode = 0555, - .child = NULL }, {} } diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 07804097f997..c4ea804d862b 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -29,9 +29,8 @@ static const struct file_operations proc_sys_dir_file_operations; static const struct inode_operations proc_sys_dir_operations; /* Support for permanently empty directories */ - struct ctl_table sysctl_mount_point[] = { - { } + {.type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY } }; /** @@ -48,21 +47,14 @@ struct ctl_table_header *register_sysctl_mount_point(const char *path) } EXPORT_SYMBOL(register_sysctl_mount_point); -static bool is_empty_dir(struct ctl_table_header *head) -{ - return head->ctl_table[0].child == sysctl_mount_point; -} - -static void set_empty_dir(struct ctl_dir *dir) -{ - dir->header.ctl_table[0].child = sysctl_mount_point; -} - -static void clear_empty_dir(struct ctl_dir *dir) - -{ - dir->header.ctl_table[0].child = NULL; -} +#define sysctl_is_perm_empty_ctl_table(tptr) \ + (tptr[0].type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) +#define sysctl_is_perm_empty_ctl_header(hptr) \ + (sysctl_is_perm_empty_ctl_table(hptr->ctl_table)) +#define sysctl_set_perm_empty_ctl_header(hptr) \ + (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) +#define sysctl_clear_perm_empty_ctl_header(hptr) \ + (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_DEFAULT) void proc_sys_poll_notify(struct ctl_table_poll *poll) { @@ -230,20 +222,22 @@ static void erase_header(struct ctl_table_header *head) static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header) { struct ctl_table *entry; + struct ctl_table_header *dir_h = &dir->header; int err; + /* Is this a permanently empty directory? */ - if (is_empty_dir(&dir->header)) + if (sysctl_is_perm_empty_ctl_header(dir_h)) return -EROFS; /* Am I creating a permanently empty directory? */ - if (header->ctl_table == sysctl_mount_point) { + if (sysctl_is_perm_empty_ctl_table(header->ctl_table)) { if (!RB_EMPTY_ROOT(&dir->root)) return -EINVAL; - set_empty_dir(dir); + sysctl_set_perm_empty_ctl_header(dir_h); } - dir->header.nreg++; + dir_h->nreg++; header->parent = dir; err = insert_links(header); if (err) @@ -259,9 +253,9 @@ fail: put_links(header); fail_links: if (header->ctl_table == sysctl_mount_point) - clear_empty_dir(dir); + sysctl_clear_perm_empty_ctl_header(dir_h); header->parent = NULL; - drop_sysctl_table(&dir->header); + drop_sysctl_table(dir_h); return err; } @@ -479,7 +473,7 @@ 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 (is_empty_dir(head)) + if (sysctl_is_perm_empty_ctl_header(head)) make_empty_dir_inode(inode); } @@ -1136,9 +1130,6 @@ static int sysctl_check_table(const char *path, struct ctl_table *table) struct ctl_table *entry; int err = 0; list_for_each_table_entry(entry, table) { - if (entry->child) - err |= sysctl_err(path, entry, "Not a file"); - if ((entry->proc_handler == proc_dostring) || (entry->proc_handler == proc_dobool) || (entry->proc_handler == proc_dointvec) || @@ -1465,25 +1456,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table, kmemleak_not_leak(hdr); } -static int count_subheaders(struct ctl_table *table) -{ - int has_files = 0; - int nr_subheaders = 0; - struct ctl_table *entry; - - /* special case: no directory and empty directory */ - if (!table || !table->procname) - return 1; - - list_for_each_table_entry(entry, table) { - if (entry->child) - nr_subheaders += count_subheaders(entry->child); - else - has_files = 1; - } - return nr_subheaders + has_files; -} - static void put_links(struct ctl_table_header *header) { struct ctl_table_set *root_set = &sysctl_table_root.default_set; @@ -1546,28 +1518,11 @@ static void drop_sysctl_table(struct ctl_table_header *header) */ void unregister_sysctl_table(struct ctl_table_header * header) { - int nr_subheaders; might_sleep(); if (header == NULL) return; - nr_subheaders = count_subheaders(header->ctl_table_arg); - if (unlikely(nr_subheaders > 1)) { - struct ctl_table_header **subheaders; - int i; - - subheaders = (struct ctl_table_header **)(header + 1); - for (i = nr_subheaders -1; i >= 0; i--) { - struct ctl_table_header *subh = subheaders[i]; - struct ctl_table *table = subh->ctl_table_arg; - unregister_sysctl_table(subh); - kfree(table); - } - kfree(header); - return; - } - spin_lock(&sysctl_lock); drop_sysctl_table(header); spin_unlock(&sysctl_lock); diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 653b66c762b1..59d451f455bf 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -137,7 +137,17 @@ struct ctl_table { void *data; int maxlen; umode_t mode; - struct ctl_table *child; /* Deprecated */ + /** + * 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; proc_handler *proc_handler; /* Callback for text formatting */ struct ctl_table_poll *poll; void *extra1; @@ -229,7 +239,7 @@ extern int unaligned_enabled; extern int unaligned_dump_stack; extern int no_unaligned_warning; -extern struct ctl_table sysctl_mount_point[]; +#define SYSCTL_PERM_EMPTY_DIR (1 << 0) #else /* CONFIG_SYSCTL */ -- cgit v1.2.3