* [PATCH 14/14] HID: check for NULL field when setting values
@ 2013-08-28 20:32 Jiri Kosina
2013-08-28 20:32 ` Jiri Kosina
2013-09-04 10:05 ` Jiri Kosina
0 siblings, 2 replies; 4+ messages in thread
From: Jiri Kosina @ 2013-08-28 20:32 UTC (permalink / raw)
To: linux-input; +Cc: Kees Cook
From: Kees Cook <keescook@chromium.org>
Defensively check that the field to be worked on is not NULL.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@kernel.org
---
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;
hid_dump_input(field->report->device, field->usage + offset, value);
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 14/14] HID: check for NULL field when setting values
2013-08-28 20:32 [PATCH 14/14] HID: check for NULL field when setting values Jiri Kosina
@ 2013-08-28 20:32 ` Jiri Kosina
2013-08-28 21:14 ` Kees Cook
2013-09-04 10:05 ` Jiri Kosina
1 sibling, 1 reply; 4+ messages in thread
From: Jiri Kosina @ 2013-08-28 20:32 UTC (permalink / raw)
To: linux-input; +Cc: Kees Cook
On Wed, 28 Aug 2013, Jiri Kosina wrote:
> From: Kees Cook <keescook@chromium.org>
>
> Defensively check that the field to be worked on is not NULL.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@kernel.org
> ---
> 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()?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 14/14] HID: check for NULL field when setting values
2013-08-28 20:32 ` Jiri Kosina
@ 2013-08-28 21:14 ` Kees Cook
0 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2013-08-28 21:14 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input
On Wed, Aug 28, 2013 at 1:32 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 28 Aug 2013, Jiri Kosina wrote:
>
>> From: Kees Cook <keescook@chromium.org>
>>
>> Defensively check that the field to be worked on is not NULL.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Cc: stable@kernel.org
>> ---
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 14/14] HID: check for NULL field when setting values
2013-08-28 20:32 [PATCH 14/14] HID: check for NULL field when setting values Jiri Kosina
2013-08-28 20:32 ` Jiri Kosina
@ 2013-09-04 10:05 ` Jiri Kosina
1 sibling, 0 replies; 4+ messages in thread
From: Jiri Kosina @ 2013-09-04 10:05 UTC (permalink / raw)
To: linux-input; +Cc: Kees Cook
On Wed, 28 Aug 2013, Jiri Kosina wrote:
> From: Kees Cook <keescook@chromium.org>
>
> Defensively check that the field to be worked on is not NULL.
Applied.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@kernel.org
> ---
> 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;
>
> hid_dump_input(field->report->device, field->usage + offset, value);
>
> --
> Jiri Kosina
> SUSE Labs
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-04 10:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28 20:32 [PATCH 14/14] HID: check for NULL field when setting values Jiri Kosina
2013-08-28 20:32 ` Jiri Kosina
2013-08-28 21:14 ` Kees Cook
2013-09-04 10:05 ` Jiri Kosina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.