Thread (7 messages) flat view 7 messages, 5 authors, 2011-10-19

Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method

From: Daniel Kurtz <hidden>
Date: 2011-10-19 07:40:40
Also in: lkml

Hi Dmitry,

I fear the following patch, and its 'Acked-by' from Nick Dyer, may
have gotten lost:

On Fri, Sep 16, 2011 at 3:40 AM, Daniel Kurtz [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Sysfs attribute show methods are always passed a buffer of length
PAGE_SIZE.  To keep from overwriting this buffer and causing havoc, use
snprintf() to guarantee we never write more than the buffer can hold.

In addition, at least for my touchscreen, the number and size of objects
was far too big to fit in a single 4K page.  Therefore, this patch also
trims some redundant framing text to leave more room for actual data.

Signed-off-by: Daniel Kurtz <redacted>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index f5d6685..a596c27 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -910,12 +910,17 @@ static ssize_t mxt_object_show(struct device *dev,
       for (i = 0; i < data->info.object_num; i++) {
               object = data->object_table + i;

-               count += sprintf(buf + count,
-                               "Object Table Element %d(Type %d)\n",
+               count += snprintf(buf + count, PAGE_SIZE - count,
+                               "Object[%d] (Type %d)\n",
                               i + 1, object->type);
+               if (count >= PAGE_SIZE)
+                       return PAGE_SIZE - 1;

               if (!mxt_object_readable(object->type)) {
-                       count += sprintf(buf + count, "\n");
+                       count += snprintf(buf + count, PAGE_SIZE - count,
+                                       "\n");
+                       if (count >= PAGE_SIZE)
+                               return PAGE_SIZE - 1;
                       continue;
               }
@@ -925,11 +930,15 @@ static ssize_t mxt_object_show(struct device *dev,
                       if (error)
                               return error;

-                       count += sprintf(buf + count,
-                                       "  Byte %d: 0x%x (%d)\n", j, val, val);
+                       count += snprintf(buf + count, PAGE_SIZE - count,
+                                       "\t[%2d]: %02x (%d)\n", j, val, val);
+                       if (count >= PAGE_SIZE)
+                               return PAGE_SIZE - 1;
               }

-               count += sprintf(buf + count, "\n");
+               count += snprintf(buf + count, PAGE_SIZE - count, "\n");
+               if (count >= PAGE_SIZE)
+                       return PAGE_SIZE - 1;
       }

       return count;
--
1.7.3.1
On Thu, Oct 6, 2011 at 5:59 PM, Nick Dyer [off-list ref] wrote:
Hi Daniel-

Just to introduce myself, I'm working for Atmel in UK on the atmel_mxt_ts
driver.

Daniel Kurtz wrote:
quoted
Sysfs attribute show methods are always passed a buffer of length
PAGE_SIZE.  To keep from overwriting this buffer and causing havoc,
use snprintf() to guarantee we never write more than the buffer can
hold.

In addition, at least for my touchscreen, the number and size of
objects was far too big to fit in a single 4K page.  Therefore, this
patch also trims some redundant framing text to leave more room for
actual data.
Yes, I'd noticed this problem as well. I was planning to address it by
splitting up the file, but your patch looks like a good solution for now.

I've tested it successfully on several chips.

Acked-by: Nick Dyer <redacted>

--
Nick Dyer
Software Engineer, ITDev  Ltd

Hardware and Software Development Consultancy

Email:   nick.dyer@itdev.co.uk
Website: http://www.itdev.co.uk
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help