On Wed, Aug 28, 2013 at 1:32 PM, Jiri Kosina <jkosina@xxxxxxx> wrote:
> On Wed, 28 Aug 2013, Jiri Kosina wrote:
>
>> From: Kees Cook <keescook@xxxxxxxxxxxx>
>>
>> Defensively check that the field to be worked on is not NULL.
>>
>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxx
>> ---
>> drivers/hid/hid-core.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 55798b2..192be6b 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1206,7 +1206,12 @@ EXPORT_SYMBOL_GPL(hid_output_report);
>>
>> int hid_set_field(struct hid_field *field, unsigned offset, __s32 value)
>> {
>> - unsigned size = field->report_size;
>> + unsigned size;
>> +
>> + if (!field)
>> + return -1;
>> +
>> + size = field->report_size;
>
> Kees,
>
> do you actually see any way how field could ever be null in
> hid_set_field()?
Yes, report->field[0] may be uninitialized (NULL due to kzalloc) if
hid_add_field() bails out during the padding field check:
if (!parser->local.usage_index) /* Ignore padding fields */
return 0;
The report will be registered (earlier call to hid_register_report()
was successful), but the entire field list will be NULL.
The hid-ntrig.c fix is a fix for such a problem
(report->field[0]->value[0]) where field[0] can be NULL. I was able to
make a reproduction case for this, with the crash can be seen in the
hid-ntrig.c patch.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
