| 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
| ||
|
Message-ID: <7d6718e0-5dfa-c688-8ae3-1cf8d0eba79c@redhat.com>
Date: Mon, 15 Aug 2022 13:11:59 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: margeyang <marge.yang@...aptics.corp-partner.google.com>,
dmitry.torokhov@...il.com, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, benjamin.tissoires@...hat.com
Cc: marge.yang@...synaptics.com, derek.cheng@...synaptics.com,
vincent.huang@...synaptics.com
Subject: Re: [PATCH V3] Input: synaptics-rmi4 - filter incomplete relative
packet.
Hi,
On 8/15/22 12:27, margeyang wrote:
> From: Marge Yang <marge.yang@...aptics.corp-partner.google.com>
>
> RMI4 F03 supports the Stick function,
> it's designed to support relative packet.
> This patch supports the following case.
> When relative packet can't be reported completely,
> it may miss one byte or two byte.
> New Synaptics firmware will report PARITY error.
> When timeout error or parity error happens,
> RMI4 driver will sends 0xFE command and
> ask FW to Re-send stick packet again.
>
> Signed-off-by: Marge Yang<marge.yang@...aptics.corp-partner.google.com>
> ---
> drivers/input/rmi4/rmi_f03.c | 77 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> index c194b1664b10..05076db83808 100644
> --- a/drivers/input/rmi4/rmi_f03.c
> +++ b/drivers/input/rmi4/rmi_f03.c
> @@ -23,8 +23,12 @@
> #define RMI_F03_BYTES_PER_DEVICE_SHIFT 4
> #define RMI_F03_QUEUE_LENGTH 0x0F
>
> +#define RMI_F03_RESET_STYK 0xFE
> +
> #define PSMOUSE_OOB_EXTRA_BTNS 0x01
>
> +#define RELATIVE_PACKET_SIZE 3
> +
> struct f03_data {
> struct rmi_function *fn;
>
> @@ -33,6 +37,11 @@ struct f03_data {
>
> unsigned int overwrite_buttons;
>
> + int iwritecommandcounter;
> + unsigned int ipacketindex;
> + unsigned int serio_flagsArry[RELATIVE_PACKET_SIZE];
> + u8 ob_dataArry[RELATIVE_PACKET_SIZE];
> +
> u8 device_count;
> u8 rx_queue_length;
> };
> @@ -88,6 +97,7 @@ static int rmi_f03_pt_write(struct serio *id, unsigned char val)
> return error;
> }
>
> + f03->iwritecommandcounter++;
> return 0;
> }
>
> @@ -107,6 +117,8 @@ static int rmi_f03_initialize(struct f03_data *f03)
> return error;
> }
>
> + f03->iwritecommandcounter = 0;
> + f03->ipacketindex = 0;
> f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
> bytes_per_device = (query1 >> RMI_F03_BYTES_PER_DEVICE_SHIFT) &
> RMI_F03_BYTES_PER_DEVICE;
> @@ -284,6 +296,22 @@ static irqreturn_t rmi_f03_attention(int irq, void *ctx)
> ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
> serio_flags = 0;
>
> + if (ob_status & (RMI_F03_OB_FLAG_TIMEOUT | RMI_F03_OB_FLAG_PARITY)) {
> + // Send resend command to stick when timeout or parity error.
> + // Driver can receive the last stick packet.
> + unsigned char val = RMI_F03_RESET_STYK;
> +
> + error = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);
> + if (error) {
> + dev_err(&f03->fn->dev,
> + "%s: Failed to rmi_write to F03 TX register (%d).\n",
> + __func__, error);
> + return error;
> + }
> + f03->ipacketindex = 0;
> + break;
> + }
> +
> if (!(ob_status & RMI_F03_RX_DATA_OFB))
> continue;
>
> @@ -298,7 +326,54 @@ static irqreturn_t rmi_f03_attention(int irq, void *ctx)
> serio_flags & SERIO_TIMEOUT ? 'Y' : 'N',
> serio_flags & SERIO_PARITY ? 'Y' : 'N');
>
> - serio_interrupt(f03->serio, ob_data, serio_flags);
> + if (f03->iwritecommandcounter > 0) {
> + // Read Acknowledge Byte after writing the PS2 command.
> + // It is not trackpoint data.
> + serio_interrupt(f03->serio, ob_data, serio_flags);
> + } else {
> + // The relative-mode PS/2 packet format is as follows:
> + //
> + // bit position position (as array of bytes)
> + // 7 6 5 4 3 2 1 0
> + // =================================+
> + // Yov Xov DY8 DX8 1 M R L | DATA[0]
> + // DX[7:0] | DATA[1]
> + // DY[7:0] | DATA[2]
> + // =================================+
> + // Yov: Y overflow
> + // Xov: X overflow
> + if ((f03->ipacketindex == 0) && (ob_data & ((BIT(7)|BIT(6))))) {
> + dev_err(&f03->fn->dev,
> + "%s: X or Y is overflow. (%x)\n",
> + __func__, ob_data);
> + break;
> + } else if ((f03->ipacketindex == 0) && !(ob_data & BIT(3))) {
> + dev_err(&f03->fn->dev,
> + "%s: New BIT 3 is not 1 for the first byte\n",
> + __func__);
> + break;
> + }
> + if (f03->ipacketindex >= RELATIVE_PACKET_SIZE)
> + f03->ipacketindex = 0;
(f03->ipacketindex >= RELATIVE_PACKET_SIZE) is never true at this point, since it
gets reset to 0 when it reaches RELATIVE_PACKET_SIZE below already.
So these 2 lines can be dropped.
Regards,
Hans
> +
> + f03->ob_dataArry[f03->ipacketindex] = ob_data;
> + f03->serio_flagsArry[f03->ipacketindex] = serio_flags;
> + f03->ipacketindex++;
> +
> + if (f03->ipacketindex == RELATIVE_PACKET_SIZE) {
> + serio_interrupt(f03->serio, f03->ob_dataArry[0],
> + f03->serio_flagsArry[0]);
> + serio_interrupt(f03->serio, f03->ob_dataArry[1],
> + f03->serio_flagsArry[1]);
> + serio_interrupt(f03->serio, f03->ob_dataArry[2],
> + f03->serio_flagsArry[2]);
> + f03->ipacketindex = 0;
> + }
> + }
> + }
> + if (f03->iwritecommandcounter > 0) {
> + f03->ipacketindex = 0;
> + f03->iwritecommandcounter = f03->iwritecommandcounter - 1;
> }
>
> return IRQ_HANDLED;
Powered by blists - more mailing lists