Consolidate debugfs mode checks in creation APIs
cover Consolidate debugfs mode checks in creation APIs
Reviewing Viresh's patch it looks like there was some ample opportunity to consolidate code here, and also close some holes where variables are modifiable. Split into multiple patches in case something causes regressions.
Stephen Boyd (4):
debugfs: Consolidate file mode checks in debugfs_create_*()
debugfs: Add read-only/write-only x64 file ops
debugfs: Add read-only/write-only size_t file ops
debugfs: Add read-only/write-only bool file ops
fs/debugfs/file.c | 123 ++++++++++++++++++++++++------------------------------ 1 file changed, 54 insertions(+), 69 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 1/4 debugfs: Consolidate file mode checks in debugfs_create_*()
The code that creates debugfs file with different file ops based on the file mode is duplicated in each debugfs_create_*() API. Consolidate that code into debugfs_create_mode(), that takes three file ops structures so that we don't have to keep copy/pasting that logic.
--- fs/debugfs/file.c | 98 ++++++++++++++++++------------------------------------- 1 file changed, 32 insertions(+), 66 deletions(-)
fs/debugfs/file.c +32 -66
@@ -42,6 +42,22 @@ const struct file_operations debugfs_file_operations = { .llseek = noop_llseek, }; +static struct dentry *debugfs_create_mode (const char *name,umode_t mode, +struct dentry *parent,void *value, +const struct file_operations *fops, +const struct file_operations *fops_ro, +const struct file_operations *fops_wo) +{ +/* if there are no write bits set, make read only */ +if (!(mode &S_IWUGO )) +return debugfs_create_file (name, mode, parent, value, fops_ro); +/* if there are no read bits set, make write only */ +if (!(mode &S_IRUGO )) +return debugfs_create_file (name, mode, parent, value, fops_wo); + +return debugfs_create_file (name, mode, parent, value, fops); +} +static int debugfs_u8_set (void *data,u64 val) { *(u8 *)data = val;
@@ -83,14 +99,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u8_wo, NULL, debugfs_u8_set, "%llu\n");struct dentry *debugfs_create_u8 (const char *name,umode_t mode,struct dentry *parent,u8 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u8_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u8_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_u8); +return debugfs_create_mode (name, mode, parent, value, &fops_u8, + &fops_u8_ro, &fops_u8_wo); }EXPORT_SYMBOL_GPL (debugfs_create_u8);
@@ -135,14 +145,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u16_wo, NULL, debugfs_u16_set, "%llu\n");struct dentry *debugfs_create_u16 (const char *name,umode_t mode,struct dentry *parent,u16 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u16_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u16_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_u16); +return debugfs_create_mode (name, mode, parent, value, &fops_u16, + &fops_u16_ro, &fops_u16_wo); }EXPORT_SYMBOL_GPL (debugfs_create_u16);
@@ -187,14 +191,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u32_wo, NULL, debugfs_u32_set, "%llu\n");struct dentry *debugfs_create_u32 (const char *name,umode_t mode,struct dentry *parent,u32 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u32_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u32_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_u32); +return debugfs_create_mode (name, mode, parent, value, &fops_u32, + &fops_u32_ro, &fops_u32_wo); }EXPORT_SYMBOL_GPL (debugfs_create_u32);
@@ -240,14 +238,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u64_wo, NULL, debugfs_u64_set, "%llu\n");struct dentry *debugfs_create_u64 (const char *name,umode_t mode,struct dentry *parent,u64 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u64_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_u64_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_u64); +return debugfs_create_mode (name, mode, parent, value, &fops_u64, + &fops_u64_ro, &fops_u64_wo); }EXPORT_SYMBOL_GPL (debugfs_create_u64);
@@ -286,14 +278,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_x64, debugfs_u64_get, debugfs_u64_set, "0x%016llx\nstruct dentry *debugfs_create_x8 (const char *name,umode_t mode,struct dentry *parent,u8 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x8_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x8_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_x8); +return debugfs_create_mode (name, mode, parent, value, &fops_x8, + &fops_x8_ro, &fops_x8_wo); }EXPORT_SYMBOL_GPL (debugfs_create_x8);
@@ -310,14 +296,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_x8);struct dentry *debugfs_create_x16 (const char *name,umode_t mode,struct dentry *parent,u16 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x16_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x16_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_x16); +return debugfs_create_mode (name, mode, parent, value, &fops_x16, + &fops_x16_ro, &fops_x16_wo); }EXPORT_SYMBOL_GPL (debugfs_create_x16);
@@ -334,14 +314,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_x16);struct dentry *debugfs_create_x32 (const char *name,umode_t mode,struct dentry *parent,u32 *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x32_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, &fops_x32_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_x32); +return debugfs_create_mode (name, mode, parent, value, &fops_x32, + &fops_x32_ro, &fops_x32_wo); }EXPORT_SYMBOL_GPL (debugfs_create_x32);
@@ -422,16 +396,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_wo, NULL, debugfs_atomic_t_set, "%lld\n");struct dentry *debugfs_create_atomic_t (const char *name,umode_t mode,struct dentry *parent,atomic_t *value) { -/* if there are no write bits set, make read only */ -if (!(mode &S_IWUGO )) -return debugfs_create_file (name, mode, parent, value, - &fops_atomic_t_ro); -/* if there are no read bits set, make write only */ -if (!(mode &S_IRUGO )) -return debugfs_create_file (name, mode, parent, value, - &fops_atomic_t_wo); - -return debugfs_create_file (name, mode, parent, value, &fops_atomic_t); +return debugfs_create_mode (name, mode, parent, value, &fops_atomic_t, + &fops_atomic_t_ro, &fops_atomic_t_wo); }EXPORT_SYMBOL_GPL (debugfs_create_atomic_t);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/4] debugfs: Consolidate file mode checks in debugfs_create_*()
Quoted 10 lines · Stephen Boyd
The code that creates debugfs file with different file ops based on the file mode is duplicated in each debugfs_create_*() API. Consolidate that code into
debugfs_create_mode(), that takes three file ops structures so that we don't have to keep copy/pasting that logic.Signed-off-by Stephen Boyd <[email protected]>--- fs/debugfs/file.c | 98 ++++++++++++++++++------------------------------------- 1 file changed, 32 insertions(+), 66 deletions(-)
Signature · 1 line
viresh
PATCH 2/4 debugfs: Add read-only/write-only x64 file ops
In reply to Stephen BoydThere aren't any read-only or write-only x64 file ops, but there is a caller of debugfs_create_x64() that calls it with mode equal to S_IRUGO. This leads to the possibility of userspace modifying the file, so let's use the newly created debugfs_create_mode() helper here to fix this.
--- fs/debugfs/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/debugfs/file.c +4 -1
@@ -256,6 +256,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_x32_ro, debugfs_u32_get, NULL, "0x%08llx\n");DEFINE_SIMPLE_ATTRIBUTE (fops_x32_wo,NULL , debugfs_u32_set,"0x%08llx\n" );DEFINE_SIMPLE_ATTRIBUTE (fops_x64, debugfs_u64_get, debugfs_u64_set,"0x%016llx\n" ); +DEFINE_SIMPLE_ATTRIBUTE (fops_x64_ro, debugfs_u64_get,NULL ,"0x%016llx\n" ); +DEFINE_SIMPLE_ATTRIBUTE (fops_x64_wo,NULL , debugfs_u64_set,"0x%016llx\n" );/* * debugfs_create_x{8,16,32,64} - create a debugfs file that is used to read and write an unsigned {8,16,32,64}-bit value
@@ -332,7 +334,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_x32);struct dentry *debugfs_create_x64 (const char *name,umode_t mode,struct dentry *parent,u64 *value) { -return debugfs_create_file (name, mode, parent, value, &fops_x64); +return debugfs_create_mode (name, mode, parent, value, &fops_x64, + &fops_x64_ro, &fops_x64_wo); }EXPORT_SYMBOL_GPL (debugfs_create_x64);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/4] debugfs: Add read-only/write-only x64 file ops
Quoted 10 lines · Stephen Boyd
There aren't any read-only or write-only x64 file ops, but there is a caller of
debugfs_create_x64()that calls it with mode equal toS_IRUGO. This leads to the possibility of userspace modifying the file, so let's use the newly createddebugfs_create_mode()helper here to fix this.Signed-off-by Stephen Boyd <[email protected]>--- fs/debugfs/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Signature · 1 line
viresh
PATCH 3/4 debugfs: Add read-only/write-only size_t file ops
In reply to Stephen BoydThere aren't any read-only or write-only size_t file ops, but there is a caller of debugfs_create_size_t() that calls it with mode equal to 0400. This leads to the possibility of userspace modifying the file, so let's use the newly created debugfs_create_mode() helper here to fix this.
--- fs/debugfs/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/debugfs/file.c +4 -1
@@ -352,6 +352,8 @@ static int debugfs_size_t_get(void *data, u64 *val) }DEFINE_SIMPLE_ATTRIBUTE (fops_size_t, debugfs_size_t_get, debugfs_size_t_set,"%llu\n" );/* %llu and %zu are more or less the same */ +DEFINE_SIMPLE_ATTRIBUTE (fops_size_t_ro, debugfs_size_t_get,NULL ,"%llu\n" ); +DEFINE_SIMPLE_ATTRIBUTE (fops_size_t_wo,NULL , debugfs_size_t_set,"%llu\n" );/** * debugfs_create_size_t - create a debugfs file that is used to read and write an size_t value
@@ -366,7 +368,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_size_t, debugfs_size_t_get, debugfs_size_t_set,struct dentry *debugfs_create_size_t (const char *name,umode_t mode,struct dentry *parent,size_t *value) { -return debugfs_create_file (name, mode, parent, value, &fops_size_t); +return debugfs_create_mode (name, mode, parent, value, &fops_size_t, + &fops_size_t_ro, &fops_size_t_wo); }EXPORT_SYMBOL_GPL (debugfs_create_size_t);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 3/4] debugfs: Add read-only/write-only size_t file ops
Quoted 10 lines · Stephen Boyd
There aren't any read-only or write-only
size_tfile ops, but there is a caller ofdebugfs_create_size_t()that calls it with mode equal to 0400. This leads to the possibility of userspace modifying the file, so let's use the newly createddebugfs_create_mode()helper here to fix this.Signed-off-by Stephen Boyd <[email protected]>--- fs/debugfs/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Signature · 1 line
viresh
PATCH 4/4 debugfs: Add read-only/write-only bool file ops
In reply to Stephen BoydThere aren't any read-only or write-only bool file ops, but there is a caller of debugfs_create_bool() that calls it with mode equal to 0400. This leads to the possibility of userspace modifying the file, so let's use the newly created debugfs_create_mode() helper here to fix this.
--- fs/debugfs/file.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
fs/debugfs/file.c +14 -1
@@ -450,6 +450,18 @@ static const struct file_operations fops_bool = { .llseek = default_llseek, }; +static const struct file_operations fops_bool_ro = { + .read = debugfs_read_file_bool, + .open = simple_open, + .llseek = default_llseek, +}; + +static const struct file_operations fops_bool_wo = { + .write = debugfs_write_file_bool, + .open = simple_open, + .llseek = default_llseek, +}; +/** * debugfs_create_bool - create a debugfs file that is used to read and write a boolean value * @name: a pointer to a string containing the name of the file to create.
@@ -477,7 +489,8 @@ static const struct file_operations fops_bool = {struct dentry *debugfs_create_bool (const char *name,umode_t mode,struct dentry *parent,bool *value) { -return debugfs_create_file (name, mode, parent, value, &fops_bool); +return debugfs_create_mode (name, mode, parent, value, &fops_bool, + &fops_bool_ro, &fops_bool_wo); }EXPORT_SYMBOL_GPL (debugfs_create_bool);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 4/4] debugfs: Add read-only/write-only bool file ops
Quoted 10 lines · Stephen Boyd
There aren't any read-only or write-only bool file ops, but there is a caller of
debugfs_create_bool()that calls it with mode equal to 0400. This leads to the possibility of userspace modifying the file, so let's use the newly createddebugfs_create_mode()helper here to fix this.Signed-off-by Stephen Boyd <[email protected]>--- fs/debugfs/file.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
Signature · 1 line
viresh