Re: [PATCH v3] w1: therm: Fix off-by-one buffer overflow in alarms_store
From: Krzysztof Kozlowski
Date: Sun Nov 09 2025 - 13:30:09 EST
On 30/10/2025 16:56, Thorsten Blum wrote:
> - /* Convert 2nd entry to int */
> - ret = kstrtoint (token, 10, &temp);
> - if (ret) {
> - dev_info(device,
> - "%s: error parsing args %d\n", __func__, ret);
> - goto free_m;
> + p = endp + 1;
> + temp = simple_strtol(p, &endp, 10);
> + if (temp < INT_MIN || temp > INT_MAX || p == endp) {
> + dev_info(device, "%s: error parsing args %d\n",
> + __func__, -EINVAL);
> + goto err;
> }
> + /* Cast to short to eliminate out of range values */
> + th = int_to_short((int)temp);
>
> - /* Prepare to cast to short by eliminating out of range values */
> - th = int_to_short(temp);
> -
> - /* Reorder if required th and tl */
> + /* Reorder if required */
> if (tl > th)
> swap(tl, th);
>
> @@ -1897,35 +1870,30 @@ static ssize_t alarms_store(struct device *device,
> * (th : byte 2 - tl: byte 3)
> */
> ret = read_scratchpad(sl, &info);
> - if (!ret) {
> - new_config_register[0] = th; /* Byte 2 */
> - new_config_register[1] = tl; /* Byte 3 */
> - new_config_register[2] = info.rom[4];/* Byte 4 */
> - } else {
> - dev_info(device,
> - "%s: error reading from the slave device %d\n",
> - __func__, ret);
> - goto free_m;
> + if (ret) {
> + dev_info(device, "%s: error reading from the slave device %d\n",
> + __func__, ret);
> + goto err;
> }
> + new_config_register[0] = th; /* Byte 2 */
> + new_config_register[1] = tl; /* Byte 3 */
> + new_config_register[2] = info.rom[4]; /* Byte 4 */
How is this change related?
>
> /* Write data in the device RAM */
> if (!SLAVE_SPECIFIC_FUNC(sl)) {
> - dev_info(device,
> - "%s: Device not supported by the driver %d\n",
> - __func__, -ENODEV);
> - goto free_m;
> + dev_info(device, "%s: Device not supported by the driver %d\n",
> + __func__, -ENODEV);
Do not introduce other formatting changes. This patch is already
difficult to read.
> + goto err;
> }
Best regards,
Krzysztof