linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eliav Farber <farbere@amazon.com>
To: <viro@zeniv.linux.org.uk>, <akpm@linux-foundation.org>,
	<yangyicong@hisilicon.com>, <linux-fsdevel@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Cc: <andriy.shevchenko@intel.com>, <farbere@amazon.com>,
	<hhhawa@amazon.com>, <jonnyc@amazon.com>
Subject: [PATCH] libfs: fix error format in simple_attr_write()
Date: Thu, 15 Sep 2022 09:15:44 +0000	[thread overview]
Message-ID: <20220915091544.42767-1-farbere@amazon.com> (raw)

In commit 488dac0c9237 ("libfs: fix error cast of negative value in
simple_attr_write()"), simple_attr_write() was changed to use kstrtoull()
instead of simple_strtoll() to convert a string got from a user.
A user trying to set a negative value will get an error.

This is wrong since it breaks all the places that use
DEFINE_DEBUGFS_ATTRIBUTE() with format of a signed integer.

For the record there are 43 current users of signed integer which are
likely to be effected by this:

$ git grep -n -A1 -w DEFINE_DEBUGFS_ATTRIBUTE | grep ');' |
sed 's,.*\(".*%.*"\).*,\1,' | sort | uniq -c
  1 "%08llx\n"
  5 "0x%016llx\n"
  5 "0x%02llx\n"
  5 "0x%04llx\n"
 13 "0x%08llx\n"
  1 "0x%4.4llx\n"
  3 "0x%.4llx\n"
  4 "0x%llx\n"
  1 "%1lld\n"
 40 "%lld\n"
  2 "%lli\n"
129 "%llu\n"
  1 "%#llx\n"
  2 "%llx\n"

u64 is not an issue for negative numbers.
The %lld and %llu in any case are for 64-bit value, representing it as
unsigned simplifies the generic code, but it doesn't mean we can't keep
their signed value if we know that.

This change uses sscanf() to fix the problem since it does the conversion
based on the supplied format string.

Fixes: 488dac0c9237 ("libfs: fix error cast of negative value in simple_attr_write()")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 fs/libfs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 31b0ddf01c31..4815c6b4fa02 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1016,9 +1016,12 @@ 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 (ret)
+	ret = sscanf(attr->set_buf, attr->fmt, &val);
+	if (ret != 1) {
+		ret = -EINVAL;
 		goto out;
+	}
+
 	ret = attr->set(attr->data, val);
 	if (ret == 0)
 		ret = len; /* on success, claim we got the whole input */
-- 
2.37.1


             reply	other threads:[~2022-09-15  9:17 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-15  9:15 Eliav Farber [this message]
2022-09-15 21:33 ` [PATCH] libfs: fix error format in simple_attr_write() Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220915091544.42767-1-farbere@amazon.com \
    --to=farbere@amazon.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=hhhawa@amazon.com \
    --cc=jonnyc@amazon.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yangyicong@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).