2022-09-19 17:45:21

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 0/3] fix error when writing negative value to simple attribute files

The simple attribute files do not accept a negative value since the
commit 488dac0c9237 ("libfs: fix error cast of negative value in
simple_attr_write()"), but some attribute files want to accept
a negative value.

Akinobu Mita (3):
libfs: add DEFINE_SIMPLE_ATTRIBUTE_SIGNED for signed value
lib/notifier-error-inject: fix error when writing -errno to debugfs
file
debugfs: fix error when writing negative value to atomic_t debugfs
file

.../fault-injection/fault-injection.rst | 10 +++----
fs/debugfs/file.c | 28 +++++++++++++++----
fs/libfs.c | 22 +++++++++++++--
include/linux/debugfs.h | 19 +++++++++++--
include/linux/fs.h | 12 ++++++--
lib/notifier-error-inject.c | 2 +-
6 files changed, 73 insertions(+), 20 deletions(-)

--
2.34.1


2022-09-19 18:07:01

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 1/3] libfs: add DEFINE_SIMPLE_ATTRIBUTE_SIGNED for signed value

The simple attribute files do not accept a negative value since the
commit 488dac0c9237 ("libfs: fix error cast of negative value in
simple_attr_write()"), so we have to use a 64-bit value to write a
negative value.

This adds DEFINE_SIMPLE_ATTRIBUTE_SIGNED for a signed value.

Fixes: 488dac0c9237 ("libfs: fix error cast of negative value in simple_attr_write()")
Reported-by: Zhao Gongyi <[email protected]>
Signed-off-by: Akinobu Mita <[email protected]>
---
fs/libfs.c | 22 +++++++++++++++++++---
include/linux/fs.h | 12 ++++++++++--
2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 31b0ddf01c31..76fb29a103a2 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -994,8 +994,8 @@ ssize_t simple_attr_read(struct file *file, char __user *buf,
EXPORT_SYMBOL_GPL(simple_attr_read);

/* interpret the buffer as a number to call the set function with */
-ssize_t simple_attr_write(struct file *file, const char __user *buf,
- size_t len, loff_t *ppos)
+static ssize_t simple_attr_write_xsigned(struct file *file, const char __user *buf,
+ size_t len, loff_t *ppos, bool is_signed)
{
struct simple_attr *attr;
unsigned long long val;
@@ -1016,7 +1016,10 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
goto out;

attr->set_buf[size] = '\0';
- ret = kstrtoull(attr->set_buf, 0, &val);
+ if (is_signed)
+ ret = kstrtoll(attr->set_buf, 0, &val);
+ else
+ ret = kstrtoull(attr->set_buf, 0, &val);
if (ret)
goto out;
ret = attr->set(attr->data, val);
@@ -1026,8 +1029,21 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
mutex_unlock(&attr->mutex);
return ret;
}
+
+ssize_t simple_attr_write(struct file *file, const char __user *buf,
+ size_t len, loff_t *ppos)
+{
+ return simple_attr_write_xsigned(file, buf, len, ppos, false);
+}
EXPORT_SYMBOL_GPL(simple_attr_write);

+ssize_t simple_attr_write_signed(struct file *file, const char __user *buf,
+ size_t len, loff_t *ppos)
+{
+ return simple_attr_write_xsigned(file, buf, len, ppos, true);
+}
+EXPORT_SYMBOL_GPL(simple_attr_write_signed);
+
/**
* generic_fh_to_dentry - generic helper for the fh_to_dentry export operation
* @sb: filesystem to do the file handle conversion on
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9eced4cc286e..c79138818922 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3471,7 +3471,7 @@ void simple_transaction_set(struct file *file, size_t n);
* All attributes contain a text representation of a numeric value
* that are accessed with the get() and set() functions.
*/
-#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
+#define DEFINE_SIMPLE_ATTRIBUTE_XSIGNED(__fops, __get, __set, __fmt, __is_signed) \
static int __fops ## _open(struct inode *inode, struct file *file) \
{ \
__simple_attr_check_format(__fmt, 0ull); \
@@ -3482,10 +3482,16 @@ static const struct file_operations __fops = { \
.open = __fops ## _open, \
.release = simple_attr_release, \
.read = simple_attr_read, \
- .write = simple_attr_write, \
+ .write = (__is_signed) ? simple_attr_write_signed : simple_attr_write, \
.llseek = generic_file_llseek, \
}

+#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
+ DEFINE_SIMPLE_ATTRIBUTE_XSIGNED(__fops, __get, __set, __fmt, false)
+
+#define DEFINE_SIMPLE_ATTRIBUTE_SIGNED(__fops, __get, __set, __fmt) \
+ DEFINE_SIMPLE_ATTRIBUTE_XSIGNED(__fops, __get, __set, __fmt, true)
+
static inline __printf(1, 2)
void __simple_attr_check_format(const char *fmt, ...)
{
@@ -3500,6 +3506,8 @@ ssize_t simple_attr_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos);
ssize_t simple_attr_write(struct file *file, const char __user *buf,
size_t len, loff_t *ppos);
+ssize_t simple_attr_write_signed(struct file *file, const char __user *buf,
+ size_t len, loff_t *ppos);

struct ctl_table;
int __init list_bdev_fs_names(char *buf, size_t size);
--
2.34.1

2022-09-19 18:08:39

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 2/3] lib/notifier-error-inject: fix error when writing -errno to debugfs file

The simple attribute files do not accept a negative value since the
commit 488dac0c9237 ("libfs: fix error cast of negative value in
simple_attr_write()").

This restores the previous behaviour by using newly introduced
DEFINE_SIMPLE_ATTRIBUTE_SIGNED instead of DEFINE_SIMPLE_ATTRIBUTE.

Fixes: 488dac0c9237 ("libfs: fix error cast of negative value in simple_attr_write()")
Reported-by: Zhao Gongyi <[email protected]>
Signed-off-by: Akinobu Mita <[email protected]>
---
lib/notifier-error-inject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/notifier-error-inject.c b/lib/notifier-error-inject.c
index 21016b32d313..2b24ea6c9497 100644
--- a/lib/notifier-error-inject.c
+++ b/lib/notifier-error-inject.c
@@ -15,7 +15,7 @@ static int debugfs_errno_get(void *data, u64 *val)
return 0;
}

-DEFINE_SIMPLE_ATTRIBUTE(fops_errno, debugfs_errno_get, debugfs_errno_set,
+DEFINE_SIMPLE_ATTRIBUTE_SIGNED(fops_errno, debugfs_errno_get, debugfs_errno_set,
"%lld\n");

static struct dentry *debugfs_create_errno(const char *name, umode_t mode,
--
2.34.1

2022-09-20 08:30:05

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 0/3] fix error when writing negative value to simple attribute files

On Tue, Sep 20, 2022 at 02:24:15AM +0900, Akinobu Mita wrote:
> The simple attribute files do not accept a negative value since the
> commit 488dac0c9237 ("libfs: fix error cast of negative value in
> simple_attr_write()"), but some attribute files want to accept
> a negative value.
>
> Akinobu Mita (3):
> libfs: add DEFINE_SIMPLE_ATTRIBUTE_SIGNED for signed value
> lib/notifier-error-inject: fix error when writing -errno to debugfs
> file
> debugfs: fix error when writing negative value to atomic_t debugfs
> file
>
> .../fault-injection/fault-injection.rst | 10 +++----
> fs/debugfs/file.c | 28 +++++++++++++++----
> fs/libfs.c | 22 +++++++++++++--
> include/linux/debugfs.h | 19 +++++++++++--
> include/linux/fs.h | 12 ++++++--
> lib/notifier-error-inject.c | 2 +-
> 6 files changed, 73 insertions(+), 20 deletions(-)

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2022-09-20 08:30:36

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 1/3] libfs: add DEFINE_SIMPLE_ATTRIBUTE_SIGNED for signed value

On 19.09.22 19:24, Akinobu Mita wrote:
> The simple attribute files do not accept a negative value since the
> commit 488dac0c9237 ("libfs: fix error cast of negative value in
> simple_attr_write()"), so we have to use a 64-bit value to write a
> negative value.
>
> This adds DEFINE_SIMPLE_ATTRIBUTE_SIGNED for a signed value.
>
> Fixes: 488dac0c9237 ("libfs: fix error cast of negative value in simple_attr_write()")

This patch itself isn't a fix. The fixes tags belong to the other
patches that make use of this.

> Reported-by: Zhao Gongyi <[email protected]>
> Signed-off-by: Akinobu Mita <[email protected]>
> ---

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2022-09-20 09:02:56

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 2/3] lib/notifier-error-inject: fix error when writing -errno to debugfs file

On 19.09.22 19:24, Akinobu Mita wrote:
> The simple attribute files do not accept a negative value since the
> commit 488dac0c9237 ("libfs: fix error cast of negative value in
> simple_attr_write()").
>
> This restores the previous behaviour by using newly introduced
> DEFINE_SIMPLE_ATTRIBUTE_SIGNED instead of DEFINE_SIMPLE_ATTRIBUTE.
>
> Fixes: 488dac0c9237 ("libfs: fix error cast of negative value in simple_attr_write()")
> Reported-by: Zhao Gongyi <[email protected]>
> Signed-off-by: Akinobu Mita <[email protected]>
> ---
> lib/notifier-error-inject.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/notifier-error-inject.c b/lib/notifier-error-inject.c
> index 21016b32d313..2b24ea6c9497 100644
> --- a/lib/notifier-error-inject.c
> +++ b/lib/notifier-error-inject.c
> @@ -15,7 +15,7 @@ static int debugfs_errno_get(void *data, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(fops_errno, debugfs_errno_get, debugfs_errno_set,
> +DEFINE_SIMPLE_ATTRIBUTE_SIGNED(fops_errno, debugfs_errno_get, debugfs_errno_set,
> "%lld\n");
>
> static struct dentry *debugfs_create_errno(const char *name, umode_t mode,

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb