linux-kernel - Re: [PATCH 7/9 v2] Input: synaptics - improved 2->3 finger transition handling

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]

Message-ID: <4E2A1ECD.3030302@canonical.com>
Date:	Fri, 22 Jul 2011 18:07:25 -0700
From:	Chase Douglas <chase.douglas@...onical.com>
To:	djkurtz@...omium.org
CC:	dmitry.torokhov@...il.com, rydberg@...omail.se,
	rubini@...l.unipv.it, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, derek.foreman@...labora.co.uk,
	daniel.stone@...labora.co.uk, olofj@...omium.org
Subject: Re: [PATCH 7/9 v2] Input: synaptics - improved 2->3 finger transition
 handling

On 07/20/2011 06:39 AM, djkurtz@...omium.org wrote:
> From: Daniel Kurtz <djkurtz@...omium.org>
> 
> As long as we know which slots are currently contained in SGM and AGM
> packets, it is possible to track the slot 0 finger when transitioning from
> 2->3 fingers.  This is the case when fingers are being added one at a
> time, 1->2->3.
> 
> However, when fingers are removed, we sometimes lose track of which slots
> are contained in SGM and AGM. In particular, when transitioning from 3->2
> and sometimes 3->1.  In both of these cases, we can no longer track slot 0
> during 2->3 transitions.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@...omium.org>
> ---
>  drivers/input/mouse/synaptics.c |   33 +++++++++++++++++++++++++++------
>  drivers/input/mouse/synaptics.h |    1 +
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index b626b98..893e567 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -659,6 +659,7 @@ static void synaptics_image_sensor_0f(struct synaptics_data *priv,
>  				      struct synaptics_mt_state *mt_state)
>  {
>  	synaptics_mt_state_set(mt_state, 0, -1, -1);
> +	priv->mt_state_lost = false;
>  }
>  
>  /* Handle case where mt_state->count = 1 */
> @@ -726,6 +727,7 @@ static void synaptics_image_sensor_1f(struct synaptics_data *priv,
>  		 * So, empty all slots. We will guess slot 0 on subsequent 1->1
>  		 */
>  		synaptics_mt_state_set(mt_state, 0, -1, -1);
> +		priv->mt_state_lost = true;
>  		break;
>  	}
>  }
> @@ -771,6 +773,7 @@ static void synaptics_image_sensor_2f(struct synaptics_data *priv,
>  		 * subsequent 2->2
>  		 */
>  		synaptics_mt_state_set(mt_state, 0, -1, -1);
> +		priv->mt_state_lost = true;
>  		break;
>  	}
>  }
> @@ -800,14 +803,32 @@ static void synaptics_image_sensor_3f(struct synaptics_data *priv,
>  		break;
>  	case 2:
>  		/*
> -		 * On 2->3 transitions, we are given no indication which finger
> -		 * was added.
> -		 * We don't even know what finger the current AGM packet
> -		 * contained.
> +		 * After some 3->1 and all 3->2 transitions, we lose track
> +		 * of which slot is reported by sgm and agm.
>  		 *
> -		 * So, empty all slots. They get filled on a subsequent 3->3
> +		 * For 2->3 in this state, empty all slots, and we will guess
> +		 * (0,1) on a subsequent 0->3.
> +		 *
> +		 * To userspace, the resulting transition will look like:
> +		 *    2:[0,1] -> 0:[-1,-1] -> 3:[0,2]

I don't think this should be allowed. We shouldn't be giving userspace
wrong information. One could argue that userspace could watch for these
transitions, but then I would argue that the driver should be handling
this :).

I don't know what the best resolution to this issue is, but any
transition in the number of fingers must be accurate. In uTouch, we
watch for touch count transitions for "continuation" gestures.

>  		 */
> -		synaptics_mt_state_set(mt_state, 0, -1, -1);
> +		if (priv->mt_state_lost) {
> +			synaptics_mt_state_set(mt_state, 0, -1, -1);
> +			break;
> +		}
> +
> +		/*
> +		 * If the (SGM,AGM) really previously contained slots (0, 1),
> +		 * then we cannot know what slot was just reported by the AGM,
> +		 * because the 2->3 transition can occur either before or after
> +		 * the AGM packet. Thus, this most recent AGM could contain
> +		 * either the same old slot 1 or the new slot 2.
> +		 * Subsequent AGMs will be reporting slot 2.
> +		 *
> +		 * To userspace, the resulting transition will look like:
> +		 *    2:[0,1] -> 1:[0,-1] -> 3:[0,2]
> +		 */
> +		synaptics_mt_state_set(mt_state, 1, 0, -1);
>  		break;
>  	case 3:
>  		/*
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index 87be1fe..e3edfea 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -162,6 +162,7 @@ struct synaptics_data {
>  	struct serio *pt_port;			/* Pass-through serio port */
>  
>  	struct synaptics_mt_state mt_state;	/* Current mt finger state */
> +	bool mt_state_lost;			/* mt_state may be incorrect */
>  
>  	/*
>  	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet

--
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/