kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: hid-debug: clean up snprintf() checks in hid_resolv_usage()
@ 2021-09-16 13:21 Dan Carpenter
  2021-09-22  9:49 ` Jiri Kosina
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-09-16 13:21 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Benjamin Tissoires, linux-input, linux-kernel, kernel-janitors

The snprintf() limits are complicated and slightly wrong when it does:

	max(0, HID_DEBUG_BUFSIZE - len - 1)

The "- 1" should not be there.  It means we can't use the last
byte of the buffer.  If we change the first snprintf() to scnprintf()
then we can remove the max().

At the start of the function the strlen(buf) is going always going to
be < HID_DEBUG_BUFSIZE so that is safe.  If it were > HID_DEBUG_BUFSIZE
then that would result in a WARN().

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/hid/hid-debug.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index fa57d05badf7..3f62fe3b0a49 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -486,8 +486,7 @@ char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 
 	if (!f) {
 		len = strlen(buf);
-		snprintf(buf+len, max(0, HID_DEBUG_BUFSIZE - len), ".");
-		len++;
+		len += scnprintf(buf + len, HID_DEBUG_BUFSIZE - len, ".");
 	}
 	else {
 		seq_printf(f, ".");
@@ -498,7 +497,7 @@ char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 				if (p->usage == (usage & 0xffff)) {
 					if (!f)
 						snprintf(buf + len,
-							max(0,HID_DEBUG_BUFSIZE - len - 1),
+							HID_DEBUG_BUFSIZE - len,
 							"%s", p->description);
 					else
 						seq_printf(f,
@@ -509,8 +508,8 @@ char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 			break;
 		}
 	if (!f)
-		snprintf(buf + len, max(0, HID_DEBUG_BUFSIZE - len - 1),
-				"%04x", usage & 0xffff);
+		snprintf(buf + len, HID_DEBUG_BUFSIZE - len, "%04x",
+			 usage & 0xffff);
 	else
 		seq_printf(f, "%04x", usage & 0xffff);
 	return buf;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] HID: hid-debug: clean up snprintf() checks in hid_resolv_usage()
  2021-09-16 13:21 [PATCH] HID: hid-debug: clean up snprintf() checks in hid_resolv_usage() Dan Carpenter
@ 2021-09-22  9:49 ` Jiri Kosina
  0 siblings, 0 replies; 2+ messages in thread
From: Jiri Kosina @ 2021-09-22  9:49 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Benjamin Tissoires, linux-input, linux-kernel, kernel-janitors

On Thu, 16 Sep 2021, Dan Carpenter wrote:

> The snprintf() limits are complicated and slightly wrong when it does:
> 
> 	max(0, HID_DEBUG_BUFSIZE - len - 1)
> 
> The "- 1" should not be there.  It means we can't use the last
> byte of the buffer.  If we change the first snprintf() to scnprintf()
> then we can remove the max().
> 
> At the start of the function the strlen(buf) is going always going to
> be < HID_DEBUG_BUFSIZE so that is safe.  If it were > HID_DEBUG_BUFSIZE
> then that would result in a WARN().

Applied, thanks Dan.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-09-22  9:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 13:21 [PATCH] HID: hid-debug: clean up snprintf() checks in hid_resolv_usage() Dan Carpenter
2021-09-22  9:49 ` Jiri Kosina

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).