[PATCH 0/4] HID: Address Coccinelle warnings

[PATCH 0/4] HID: Address Coccinelle warnings

[Chen Changcheng]

This series addresses four Coccinelle-detected code style issues in
the HID subsystem:

- Replace open-coded min() logic with the standard min() macro (3 files)
- Use secs_to_jiffies() instead of msecs_to_jiffies(sec * MSEC_PER_SEC)

All changes are straightforward, no functional changes intended.

./hid-sony.c:970:21-23: WARNING opportunity for min()
./hid-picolcd_debugfs.c:89:17-18: WARNING opportunity for min()
./intel-thc-hid/intel-quicki2c/pci-quicki2c.c:213:22-24: WARNING opportunity for min()
./intel-thc-hid/intel-quicki2c/pci-quicki2c.c:218:22-24: WARNING opportunity for min()
./intel-ish-hid/ishtp/ishtp-dev.h:272:25-28: WARNING opportunity for secs_to_jiffies()

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
Chen Changcheng (4):
  HID: intel-quicki2c: Use min() macro to simplify code
  HID: sony: Use min() macro to simplify code
  HID: picolcd: Use min() macro to simplify code
  HID: intel-ish-hid: Use secs_to_jiffies() instead of
    msecs_to_jiffies(sec * MSEC_PER_SEC)

 drivers/hid/hid-picolcd_debugfs.c                     |  2 +-
 drivers/hid/hid-sony.c                                |  2 +-
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h           |  2 +-
 .../hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c   | 11 ++---------
 4 files changed, 5 insertions(+), 12 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.25.1

[PATCH 1/4] HID: intel-quicki2c: Use min() macro to simplify code

[Chen Changcheng]

Replace open-coded min() logic with the standard min() macro in
pci-quicki2c.c for setting i2c_max_frame_size and i2c_int_delay.
This improves code readability and is consistent with kernel coding
style.

Detected by Coccinelle:
./intel-thc-hid/intel-quicki2c/pci-quicki2c.c:213:22-24: WARNING opportunity for min()
./intel-thc-hid/intel-quicki2c/pci-quicki2c.c:218:22-24: WARNING opportunity for min()

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 .../hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c   | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 46d3e9a01999..bd099600bb9f 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -210,15 +210,8 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
 		qcdev->i2c_max_frame_size_enable = i2c_config.FSEN;
 		qcdev->i2c_int_delay_enable = i2c_config.INDE;
 
-		if (i2c_config.FSVL <= qcdev->ddata->max_detect_size)
-			qcdev->i2c_max_frame_size = i2c_config.FSVL;
-		else
-			qcdev->i2c_max_frame_size = qcdev->ddata->max_detect_size;
-
-		if (i2c_config.INDV <= qcdev->ddata->max_interrupt_delay)
-			qcdev->i2c_int_delay = i2c_config.INDV;
-		else
-			qcdev->i2c_int_delay = qcdev->ddata->max_interrupt_delay;
+		qcdev->i2c_max_frame_size = min(i2c_config.FSVL, qcdev->ddata->max_detect_size);
+		qcdev->i2c_int_delay = min(i2c_config.INDV, qcdev->ddata->max_interrupt_delay);
 	}
 
 	return 0;
-- 
2.25.1

Re: [PATCH 1/4] HID: intel-quicki2c: Use min() macro to simplify code

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the helper function `quicki2c_acpi_get_dsd_property()` fails to validate the ACPI object type, performs an unbounded memory copy, and leaks the dynamically allocated ACPI buffer.
--

--- Patch [1]: [PATCH 1/4] HID: intel-quicki2c: Use min() macro to simplify code ---
commit 232da0a8d999e7cca061120fe2a92fd5ce93fd4e
Author: Chen Changcheng <chenchangcheng@kylinos.cn>
Subject: HID: intel-quicki2c: Use min() macro to simplify code

This patch replaces open-coded min() logic with the standard min() macro in
pci-quicki2c.c. This is used for setting i2c_max_frame_size and i2c_int_delay
to improve code readability and maintain consistency with kernel style.

> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> index 46d3e9a019995..bd099600bb9f2 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -210,15 +210,8 @@ static int quicki2c_get_acpi_resources(struct quicki2c_device *qcdev)
>  		qcdev->i2c_max_frame_size_enable = i2c_config.FSEN;
>  		qcdev->i2c_int_delay_enable = i2c_config.INDE;
>  

[Severity: High]
This isn't a bug introduced by this patch, but since this code relies on 
i2c_config populated earlier by quicki2c_acpi_get_dsd_property(), does the 
helper function validate the ACPI object type before copying it?

Looking at quicki2c_acpi_get_dsd_property():

quicki2c_acpi_get_dsd_property(...)
{
        ...
        memcpy(prop_buf, ret_obj->buffer.pointer, ret_obj->buffer.length);
        ...
        return 0;
}

Does this memcpy blindly use the length from the ACPI object without
ensuring it fits into the destination buffer (such as the stack-allocated
i2c_config)? If firmware provides a larger buffer, could this overflow the
stack frame in quicki2c_get_acpi_resources()?

Also, if ret_obj->type is not verified as a buffer, could this cause an
invalid pointer dereference since the buffer pointer might overlap with
an integer value?

Additionally, does the success path in quicki2c_acpi_get_dsd_property()
leak the dynamically allocated ACPI buffer by returning without calling
ACPI_FREE(buffer.pointer)?

> -		if (i2c_config.FSVL <= qcdev->ddata->max_detect_size)
> -			qcdev->i2c_max_frame_size = i2c_config.FSVL;
> -		else
> -			qcdev->i2c_max_frame_size = qcdev->ddata->max_detect_size;
> -
> -		if (i2c_config.INDV <= qcdev->ddata->max_interrupt_delay)
> -			qcdev->i2c_int_delay = i2c_config.INDV;
> -		else
> -			qcdev->i2c_int_delay = qcdev->ddata->max_interrupt_delay;
> +		qcdev->i2c_max_frame_size = min(i2c_config.FSVL, qcdev->ddata->max_detect_size);
> +		qcdev->i2c_int_delay = min(i2c_config.INDV, qcdev->ddata->max_interrupt_delay);
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728025750.48249-1-chenchangcheng@kylinos.cn?part=1

[PATCH 2/4] HID: sony: Use min() macro to simplify code

[Chen Changcheng]

Replace open-coded min() logic with the standard min() macro
in hid-sony.c for battery capacity index calculation.

Detected by Coccinelle:
./hid-sony.c:970:21-23: WARNING opportunity for min()

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/hid-sony.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index e75246d29e16..cbc916989646 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -967,7 +967,7 @@ static void sixaxis_parse_report(struct sony_sc *sc, u8 *rd, int size)
 		battery_capacity = 100;
 		battery_status = (rd[offset] & 0x01) ? POWER_SUPPLY_STATUS_FULL : POWER_SUPPLY_STATUS_CHARGING;
 	} else {
-		index = rd[offset] <= 5 ? rd[offset] : 5;
+		index = min(rd[offset], 5);
 		battery_capacity = sixaxis_battery_capacity[index];
 		battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
 	}
-- 
2.25.1

[PATCH 3/4] HID: picolcd: Use min() macro to simplify code

[Chen Changcheng]

Replace open-coded min() logic with the standard min() macro
in hid-picolcd_debugfs.c for limiting read size.

Detected by Coccinelle:
./hid-picolcd_debugfs.c:89:17-18: WARNING opportunity for min()

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/hid-picolcd_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
index 085847a92e07..c5e0e9076de9 100644
--- a/drivers/hid/hid-picolcd_debugfs.c
+++ b/drivers/hid/hid-picolcd_debugfs.c
@@ -86,7 +86,7 @@ static ssize_t picolcd_debug_eeprom_read(struct file *f, char __user *u,
 	/* prepare buffer with info about what we want to read (addr & len) */
 	raw_data[0] = *off & 0xff;
 	raw_data[1] = (*off >> 8) & 0xff;
-	raw_data[2] = s < 20 ? s : 20;
+	raw_data[2] = min(s, 20);
 	if (*off + raw_data[2] > 0xff)
 		raw_data[2] = 0x100 - *off;
 	resp = picolcd_send_and_wait(data->hdev, REPORT_EE_READ, raw_data,
-- 
2.25.1

Re: [PATCH 3/4] HID: picolcd: Use min() macro to simplify code

[sashiko-bot]

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

Pre-existing issues:
- [High] Out-of-bounds read in `picolcd_debug_eeprom_read` leaking kernel heap memory to userspace.
--

--- Patch [3]: [PATCH 3/4] HID: picolcd: Use min() macro to simplify code ---
commit 1474980d788a9366218f25c39b115f1aca750b68
Author: Chen Changcheng <chenchangcheng@kylinos.cn>

HID: picolcd: Use min() macro to simplify code

This commit replaces an open-coded minimum check with the standard min()
macro in hid-picolcd_debugfs.c. It simplifies the logic for limiting the
read size for EEPROM data.

> diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
> --- a/drivers/hid/hid-picolcd_debugfs.c
> +++ b/drivers/hid/hid-picolcd_debugfs.c
> @@ -86,7 +86,7 @@ static ssize_t picolcd_debug_eeprom_read(struct file *f, char __user *u,
>  	/* prepare buffer with info about what we want to read (addr & len) */
>  	raw_data[0] = *off & 0xff;
>  	raw_data[1] = (*off >> 8) & 0xff;
> -	raw_data[2] = s < 20 ? s : 20;
> +	raw_data[2] = min(s, 20);

[ ... ]

> 	if (resp->in_report && resp->in_report->id == REPORT_EE_DATA) {
> 		/* successful read :) */
> 		ret = resp->raw_data[2];
> 		if (ret > s)
> 			ret = s;
> 		if (copy_to_user(u, resp->raw_data+3, ret))

[Severity: High]
This is a pre-existing issue, but while reviewing this function, can a
malicious or buggy USB device cause an out-of-bounds read here?

If a user reads from the eeprom file with a large size, for example 
s = 256, and a malicious device responds with a REPORT_EE_DATA containing
a large embedded length field in resp->raw_data[2] up to 255, ret will take
that device-controlled value.

Since ret (255) would not be greater than s (256), this copy_to_user()
call could attempt to copy up to 255 bytes. 

Given that resp->raw_data is a fixed 64-byte array, does this allow reading
past the end of the struct picolcd_pending heap allocation and leaking
adjacent kernel heap memory to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728025750.48249-1-chenchangcheng@kylinos.cn?part=3

[PATCH 4/4] HID: intel-ish-hid: Use secs_to_jiffies() instead of msecs_to_jiffies(sec * MSEC_PER_SEC)

[Chen Changcheng]

Use the standard secs_to_jiffies() helper instead of open-coding
the conversion from seconds via msecs_to_jiffies(). This is more
efficient and improves code readability.

Detected by Coccinelle:
./intel-ish-hid/ishtp/ishtp-dev.h:272:25-28: WARNING opportunity for secs_to_jiffies()

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index 4b0596eadf1c..9457c30b1507 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -269,7 +269,7 @@ struct ishtp_device {
 
 static inline unsigned long ishtp_secs_to_jiffies(unsigned long sec)
 {
-	return msecs_to_jiffies(sec * MSEC_PER_SEC);
+	return secs_to_jiffies(sec);
 }
 
 /*
-- 
2.25.1