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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGS+omBkpTi34diCrp_99FPHaTpTT35baX6veOX2+B-RhR6QqQ@mail.gmail.com>
Date:	Fri, 16 Sep 2011 13:10:29 +0800
From:	Daniel Kurtz <djkurtz@...omium.org>
To:	Joonyoung Shim <jy0922.shim@...sung.com>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Iiro Valkonen <iiro.valkonen@...el.com>,
	Henrik Rydberg <rydberg@...omail.se>,
	Yufeng Shen <miletus@...omium.org>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute
 show method

Hi Joonyoung,

Thanks for replying.

On Fri, Sep 16, 2011 at 8:52 AM, Joonyoung Shim <jy0922.shim@...sung.com> wrote:
> Hi,
>
> On 2011-09-16 오전 4:40, Daniel Kurtz wrote:
>>
>> 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<djkurtz@...omium.org>
>
> This patch is working but original problem is object size is increasing
> as atmel touchscreen chip updates and we should consider also the
> instances of object.

"This patch is working" = Acked-By ?

> How about splitting attributes to each object?

I agree.  Can we do that in another patch (with instance printing)?

> Iiro, do you have any update plans about atmel touchscreen driver?

I have several more patches coming for this driver, mostly to speed up
startup time, cleanup the MT-B implementation, and optimize interrupt
handling.

Thanks,
Daniel

>> ---
>>  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;