On Wed, 2019-01-09 at 13:50 +1000, Peter Hutterer wrote:
> Introduced in c53431eb696f3c64c12c00afb81048af54b61532
> "HID: core: store the collections as a basic tree".
>
> Previously, the pointer to the parent collection was stored. If a
> device
> exceeds 16 collections (HID_DEFAULT_NUM_COLLECTIONS), the array to
> store
> the collections is reallocated, the pointer to the parent collection
> becomes
> invalid.
>
> Replace the pointers with an index-based lookup into the collections
> array.
>
> Reported-by: Pandruvada, Srinivas <srinivas.pandruvada@xxxxxxxxx>
> Signed-off-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
Added Kyle to this email, who is testing this change.
Thanks,
Srinivas
> ---
> I have a test locally for hid-tools now but it relies on the kernel
> crashing, so it's going to generate false positives. I verified the
> test
> with printks though.
>
> drivers/hid/hid-core.c | 32 +++++++++++++++++++++-----------
> include/linux/hid.h | 4 ++--
> 2 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index f41d5fe51abe..f9093dedf647 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -125,6 +125,7 @@ static int open_collection(struct hid_parser
> *parser, unsigned type)
> {
> struct hid_collection *collection;
> unsigned usage;
> + int collection_index;
>
> usage = parser->local.usage[0];
>
> @@ -167,13 +168,13 @@ static int open_collection(struct hid_parser
> *parser, unsigned type)
> parser->collection_stack[parser->collection_stack_ptr++] =
> parser->device->maxcollection;
>
> - collection = parser->device->collection +
> - parser->device->maxcollection++;
> + collection_index = parser->device->maxcollection++;
> + collection = parser->device->collection + collection_index;
> collection->type = type;
> collection->usage = usage;
> collection->level = parser->collection_stack_ptr - 1;
> - collection->parent = parser->active_collection;
> - parser->active_collection = collection;
> + collection->parent_idx = parser->active_collection_idx;
> + parser->active_collection_idx = collection_index;
>
> if (type == HID_COLLECTION_APPLICATION)
> parser->device->maxapplication++;
> @@ -192,8 +193,13 @@ static int close_collection(struct hid_parser
> *parser)
> return -EINVAL;
> }
> parser->collection_stack_ptr--;
> - if (parser->active_collection)
> - parser->active_collection = parser->active_collection-
> >parent;
> + if (parser->active_collection_idx != -1) {
> + struct hid_device *device = parser->device;
> + struct hid_collection *c;
> +
> + c = &device->collection[parser->active_collection_idx];
> + parser->active_collection_idx = c->parent_idx;
> + }
> return 0;
> }
>
> @@ -819,6 +825,7 @@ static int hid_scan_report(struct hid_device
> *hid)
> return -ENOMEM;
>
> parser->device = hid;
> + parser->active_collection_idx = -1;
> hid->group = HID_GROUP_GENERIC;
>
> /*
> @@ -1006,10 +1013,12 @@ static void
> hid_apply_multiplier_to_field(struct hid_device *hid,
> usage = &field->usage[i];
>
> collection = &hid->collection[usage->collection_index];
> - while (collection && collection !=
> multiplier_collection)
> - collection = collection->parent;
> + while (collection->parent_idx != -1 &&
> + collection != multiplier_collection)
> + collection = &hid->collection[collection-
> >parent_idx];
>
> - if (collection || multiplier_collection == NULL)
> + if (collection->parent_idx != -1 ||
> + multiplier_collection == NULL)
> usage->resolution_multiplier =
> effective_multiplier;
>
> }
> @@ -1044,9 +1053,9 @@ static void hid_apply_multiplier(struct
> hid_device *hid,
> * applicable fields later.
> */
> multiplier_collection = &hid->collection[multiplier->usage-
> >collection_index];
> - while (multiplier_collection &&
> + while (multiplier_collection->parent_idx != -1 &&
> multiplier_collection->type != HID_COLLECTION_LOGICAL)
> - multiplier_collection = multiplier_collection->parent;
> + multiplier_collection = &hid-
> >collection[multiplier_collection->parent_idx];
>
> effective_multiplier = hid_calculate_multiplier(hid,
> multiplier);
>
> @@ -1170,6 +1179,7 @@ int hid_open_report(struct hid_device *device)
> }
>
> parser->device = device;
> + parser->active_collection_idx = -1;
>
> end = start + size;
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 93db548f8761..554e82d812da 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -429,7 +429,7 @@ struct hid_local {
> */
>
> struct hid_collection {
> - struct hid_collection *parent;
> + int parent_idx; /* device->collection */
> unsigned type;
> unsigned usage;
> unsigned level;
> @@ -657,7 +657,7 @@ struct hid_parser {
> unsigned int *collection_stack;
> unsigned int collection_stack_ptr;
> unsigned int collection_stack_size;
> - struct hid_collection *active_collection;
> + int active_collection_idx; /* device-
> >collection */
> struct hid_device *device;
> unsigned int scan_flags;
> };
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
