linux-kernel - Re: [PATCH v2 1/4] hid-multitouch: Auto detection of maxcontacts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 10 Mar 2011 13:45:30 +0100
From:	"Henrik Rydberg" <rydberg@...omail.se>
To:	Benjamin Tissoires <benjamin.tissoires@...c.fr>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...-enac.fr>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] hid-multitouch: Auto detection of maxcontacts

Hi Benjamin,

thanks for this, just a couple of things...

> @@ -126,9 +127,19 @@ struct mt_class mt_classes[] = {
>  static void mt_feature_mapping(struct hid_device *hdev,
>  		struct hid_field *field, struct hid_usage *usage)
>  {
> -	if (usage->hid == HID_DG_INPUTMODE) {
> -		struct mt_device *td = hid_get_drvdata(hdev);
> +	struct mt_device *td = hid_get_drvdata(hdev);
> +
> +	switch (usage->hid) {
> +	case HID_DG_INPUTMODE:
>  		td->inputmode = field->report->id;
> +		break;
> +	case HID_DG_CONTACTMAX:
> +		td->maxcontacts = *(field->value);

field->value[0] seems more standard-ish

> +		if (td->mtclass->maxcontacts > td->maxcontacts)
> +			/* check if the maxcontacts is given by the class */
> +			td->maxcontacts = td->mtclass->maxcontacts;
> +
> +		break;
>  	}
>  }
>  
> @@ -317,7 +327,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>  	struct mt_device *td = hid_get_drvdata(hid);
>  	__s32 quirks = td->mtclass->quirks;
>  
> -	if (hid->claimed & HID_CLAIMED_INPUT) {
> +	if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {

The extra test could in principle be placed after the usage switch, which would prevent partially filled contact data, should this ever occur. Something like

if (!td->slots)
   break;

>  		switch (usage->hid) {
>  		case HID_DG_INRANGE:
>  			if (quirks & MT_QUIRK_VALID_IS_INRANGE)
> @@ -415,9 +425,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	 */
>  	hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>  
> -	td = kzalloc(sizeof(struct mt_device) +
> -				mtclass->maxcontacts * sizeof(struct mt_slot),
> -				GFP_KERNEL);
> +	td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
>  	if (!td) {
>  		dev_err(&hdev->dev, "cannot allocate multitouch data\n");
>  		return -ENOMEM;
> @@ -434,6 +442,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (ret)
>  		goto fail;
>  
> +	td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> +				GFP_KERNEL);
> +	if (!td->slots) {
> +		dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
> +		ret = -ENOMEM;
> +		goto fail;

This exit leaves the device running.

> +	}
> +
>  	mt_set_input_mode(hdev);
>  
>  	return 0;
> @@ -455,6 +471,7 @@ static void mt_remove(struct hid_device *hdev)
>  {
>  	struct mt_device *td = hid_get_drvdata(hdev);
>  	hid_hw_stop(hdev);
> +	kfree(td->slots);
>  	kfree(td);
>  	hid_set_drvdata(hdev, NULL);
>  }
> -- 
> 1.7.4
> 

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/