Linux Input/HID development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "David Heidelberg" <david@ixit.cz>
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org
Subject: Re: [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12
Date: Fri, 31 Jul 2026 18:25:52 +0000	[thread overview]
Message-ID: <20260731182552.9E9EA1F00AC4@smtp.kernel.org> (raw)
In-Reply-To: <20260731-respect-x-y-mm-v1-1-3e85a4bec745@ixit.cz>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential division by zero if the device hardware reports a DPM resolution of 0.
- [High] Uninitialized kernel stack memory leak due to insufficient validation of register size against subpacket map.
--

--- Patch [1]: [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 ---
commit b9eac21396bb125557501cbe3a56f93e5854436c
Author: David Heidelberg <david@ixit.cz>

Input: rmi4 - report difference between platform data and detected coord in F12

Platform data may define touchscreen-x-mm and touchscreen-y-mm, but
these are quietly overridden by data provided by sensor.

Warn when detected data doesn't match the device-tree data.

> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 88c28089de993..88797556e738c 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -100,6 +100,7 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
>  	int ret;
>  	int offset;
>  	u8 buf[15];
> +	u8 x_mm, y_mm;

[Severity: High]
This isn't a bug introduced by this patch, but could this code leak
uninitialized stack memory if a peripheral advertises a small register size?

In rmi_f12_read_sensor_tuning(), buf is populated via rmi_read_block() with
exactly item->reg_size bytes. Later in the function, it unconditionally
parses fields if the corresponding subpacket bit is set:

rmi_f12_read_sensor_tuning()
    ...
    offset = 0;
    if (rmi_register_desc_has_subpacket(item, 0)) {
        sensor->max_x = get_unaligned_le16(&buf[offset]);
        sensor->max_y = get_unaligned_le16(&buf[offset + 2]);
    ...

If a peripheral sends a crafted register descriptor with a small reg_size
(e.g., 2 bytes) but sets bits in the subpacket map, does this cause the
driver to read past the initialized portion of buf and expose stack memory
to userspace?

>  	int pitch_x = 0;
>  	int pitch_y = 0;
>  	int rx_receivers = 0;

[ ... ]

> @@ -171,8 +172,8 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
>  		}
>  		dpm_resolution = buf[0];
>  
> -		sensor->x_mm = sensor->max_x / dpm_resolution;
> -		sensor->y_mm = sensor->max_y / dpm_resolution;
> +		x_mm = sensor->max_x / dpm_resolution;
> +		y_mm = sensor->max_y / dpm_resolution;

[Severity: High]
This is a pre-existing issue, but is it possible for a malicious or
malfunctioning device to cause a division by zero panic here?

The dpm_resolution value is read directly from the hardware query register
into buf[0] and then used unconditionally as the denominator:

    dpm_resolution = buf[0];
    x_mm = sensor->max_x / dpm_resolution;

If the unprivileged peripheral returns a value of 0, could this crash the
entire system?

>  	} else {
>  		if (rmi_register_desc_has_subpacket(item, 3)) {
>  			rx_receivers = buf[offset];

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-respect-x-y-mm-v1-0-3e85a4bec745@ixit.cz?part=1

  reply	other threads:[~2026-07-31 18:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 18:13 [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm David Heidelberg via B4 Relay
2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
2026-07-31 18:25   ` sashiko-bot [this message]
2026-07-31 18:13 ` [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm " David Heidelberg via B4 Relay
2026-07-31 18:23   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260731182552.9E9EA1F00AC4@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=david@ixit.cz \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox