Re: [PATCH 1/1] drm/i915: shorten i915_next_seqno debugfs output

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Kees Cook <keescook@chromium.org> writes:

> On Thu, Apr 11, 2013 at 9:15 AM, Mika Kuoppala
> <mika.kuoppala@linux.intel.com> wrote:
>> Kees Cook <keescook@chromium.org> writes:
>>
>>> On Thu, Apr 11, 2013 at 6:22 AM, Mika Kuoppala
>>> <mika.kuoppala@linux.intel.com> wrote:
>>>> commit 647416f9eefe7699754b01b9fc82758fde83248c
>>>> Author: Kees Cook <keescook@chromium.org>
>>>> Date:   Sun Mar 10 14:10:06 2013 -0700
>>>>
>>>>     drm/i915: use simple attribute in debugfs routines
>>>>
>>>> made i915_next_seqno debugfs entry to crop it's output
>>>> if returned value was large enough. Using simple_attr
>>>> will limit the output to 24 bytes. Fix this by returning
>>>> only the value and nothing else.
>>>>
>>>> Cc: Kees Cook <keescook@chromium.org>
>>>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>>>
>>> Oh! Thanks for catching that. What a weird limitation.
>>>
>>> What about max freq, min freq, and wedged? Do those run the risk of
>>> truncation too?
>>
>> max and min freq should be safe, and wedged too on 32bit platforms.
>> But if gpu is declared wedged on host with 64bit atomic_t,
>> it will crop the output.
>
> Should we consider proposing an increase in the size of the simple
> attr buffer? It seems silly to provide an arbitrary format string but
> leave the buffer so small.

That or dynamic allocation to attr->get_buf. But that would need
extra pass with snprintf and I don't know if that qualifies as 'simple'
anymore.

I have posted a patch which fixes all i915 debugfs simple attributes
to fit into the simple_attr by removing everything except the fmt to
value. I didn't find any testcases which would rely on preample
being there.

--Mika