Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1245239 > unrolled thread

[PATCH 0/4] Consolidate debugfs mode checks in creation APIs

Started byStephen Boyd <sboyd@codeaurora.org>
First post2015-10-13 03:10 +0200
Last post2015-10-13 03:20 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] Consolidate debugfs mode checks in creation APIs Stephen Boyd <sboyd@codeaurora.org> - 2015-10-13 03:10 +0200
    [PATCH 1/4] debugfs: Consolidate file mode checks in debugfs_create_*() Stephen Boyd <sboyd@codeaurora.org> - 2015-10-13 03:10 +0200
    [PATCH 2/4] debugfs: Add read-only/write-only x64 file ops Stephen Boyd <sboyd@codeaurora.org> - 2015-10-13 03:10 +0200
    [PATCH 4/4] debugfs: Add read-only/write-only bool file ops Stephen Boyd <sboyd@codeaurora.org> - 2015-10-13 03:10 +0200
      Re: [PATCH 4/4] debugfs: Add read-only/write-only bool file ops Viresh Kumar <viresh.kumar@linaro.org> - 2015-10-13 05:50 +0200
    [PATCH 3/4] debugfs: Add read-only/write-only size_t file ops Stephen Boyd <sboyd@codeaurora.org> - 2015-10-13 03:20 +0200

#1245239 — [PATCH 0/4] Consolidate debugfs mode checks in creation APIs

FromStephen Boyd <sboyd@codeaurora.org>
Date2015-10-13 03:10 +0200
Subject[PATCH 0/4] Consolidate debugfs mode checks in creation APIs
Message-ID<qiWgi-7NK-1@gated-at.bofh.it>
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(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1245241 — [PATCH 1/4] debugfs: Consolidate file mode checks in debugfs_create_*()

FromStephen Boyd <sboyd@codeaurora.org>
Date2015-10-13 03:10 +0200
Subject[PATCH 1/4] debugfs: Consolidate file mode checks in debugfs_create_*()
Message-ID<qiWgi-7NK-7@gated-at.bofh.it>
In reply to#1245239
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 <sboyd@codeaurora.org>
---
 fs/debugfs/file.c | 98 ++++++++++++++++++-------------------------------------
 1 file changed, 32 insertions(+), 66 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index b70c20fae502..5d594efa7c93 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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\n
 struct 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);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245242 — [PATCH 2/4] debugfs: Add read-only/write-only x64 file ops

FromStephen Boyd <sboyd@codeaurora.org>
Date2015-10-13 03:10 +0200
Subject[PATCH 2/4] debugfs: Add read-only/write-only x64 file ops
Message-ID<qiWgi-7NK-9@gated-at.bofh.it>
In reply to#1245239
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
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.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 fs/debugfs/file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 5d594efa7c93..f69d42efe4b8 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245246 — [PATCH 4/4] debugfs: Add read-only/write-only bool file ops

FromStephen Boyd <sboyd@codeaurora.org>
Date2015-10-13 03:10 +0200
Subject[PATCH 4/4] debugfs: Add read-only/write-only bool file ops
Message-ID<qiWgj-7NK-27@gated-at.bofh.it>
In reply to#1245239
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 created
debugfs_create_mode() helper here to fix this.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 fs/debugfs/file.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index e8e73aebe6b8..8450549d54a9 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245320 — Re: [PATCH 4/4] debugfs: Add read-only/write-only bool file ops

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-10-13 05:50 +0200
SubjectRe: [PATCH 4/4] debugfs: Add read-only/write-only bool file ops
Message-ID<qiYL7-2Uy-1@gated-at.bofh.it>
In reply to#1245246
On 12-10-15, 18:09, Stephen Boyd wrote:
> 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 created
> debugfs_create_mode() helper here to fix this.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  fs/debugfs/file.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245255 — [PATCH 3/4] debugfs: Add read-only/write-only size_t file ops

FromStephen Boyd <sboyd@codeaurora.org>
Date2015-10-13 03:20 +0200
Subject[PATCH 3/4] debugfs: Add read-only/write-only size_t file ops
Message-ID<qiWpY-7YV-15@gated-at.bofh.it>
In reply to#1245239
There 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.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 fs/debugfs/file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index f69d42efe4b8..e8e73aebe6b8 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web