lkml.org 
[lkml]   [2012]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] HID: leave dev_rdesc unmodified and use it for comparisons
On 09/19/12 01:05 N.U., Jiri Slaby wrote:
> AFAICS this is incorrect. Some drivers return pointers to their own
> static structure from their .report_fixup. Hence there are two problems:
> * leak, because kmemdup'ped start is never freed
> * invalid free -- kfree(device->rdesc) will try to free a static
> structure regards,

On 09/19/12 04:55 N.U., Jiri Kosina wrote:
> How do you avoid memory leak on 'start' here?

Hmm. I hadn't noticed that the other drivers are returning a static
structure. In that case, it seems that report_fixup itself is broken
from a memory perspective, in that it returns pointers to inconsistent
storage types depending on the driver. (As evidenced by how my first
patch version would have also caused invalid frees that weren't evident
from the declaration in hid.h.) I see two options:

1. Ugly workaround: make a temporary copy of the dev_rdesc, give it to
report_fixup, make a copy of the return, store that copy in rdesc, free
the temporary copy. Though ugly, this would at least involve the
smallest diff.

2. Standardize the behavior of the drivers' report_fixup
implementations. Given that some of them need to change the size of the
descriptor, modifying the passed structure is not an option. Probably
all of them should return a newly allocated structure, either a modified
copy of the input or a copy of their static, that can then be stored
directly in rdesc. Especially since report_fixup is only ever called
right before a copy is going to be taken anyway. (Adding constness to a
parameter isn't considered a severe ABI break, is it?)

Thoughts?

-Kevin


\
 
 \ /
  Last update: 2012-09-19 19:41    [W:0.056 / U:1.852 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site