Skip to content

Commit 761fcf4

Browse files
tobluxkrzk
authored andcommitted
w1: therm: Fix off-by-one buffer overflow in alarms_store
The sysfs buffer passed to alarms_store() is allocated with 'size + 1' bytes and a NUL terminator is appended. However, the 'size' argument does not account for this extra byte. The original code then allocated 'size' bytes and used strcpy() to copy 'buf', which always writes one byte past the allocated buffer since strcpy() copies until the NUL terminator at index 'size'. Fix this by parsing the 'buf' parameter directly using simple_strtoll() without allocating any intermediate memory or string copying. This removes the overflow while simplifying the code. Cc: stable@vger.kernel.org Fixes: e2c94d6 ("w1_therm: adding alarm sysfs entry") Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Link: https://patch.msgid.link/20251216145007.44328-2-thorsten.blum@linux.dev Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
1 parent 8f0b4cc commit 761fcf4

1 file changed

Lines changed: 20 additions & 42 deletions

File tree

drivers/w1/slaves/w1_therm.c

Lines changed: 20 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,53 +1836,35 @@ static ssize_t alarms_store(struct device *device,
18361836
struct w1_slave *sl = dev_to_w1_slave(device);
18371837
struct therm_info info;
18381838
u8 new_config_register[3]; /* array of data to be written */
1839-
int temp, ret;
1840-
char *token = NULL;
1839+
long long temp;
1840+
int ret = 0;
18411841
s8 tl, th; /* 1 byte per value + temp ring order */
1842-
char *p_args, *orig;
1843-
1844-
p_args = orig = kmalloc(size, GFP_KERNEL);
1845-
/* Safe string copys as buf is const */
1846-
if (!p_args) {
1847-
dev_warn(device,
1848-
"%s: error unable to allocate memory %d\n",
1849-
__func__, -ENOMEM);
1850-
return size;
1851-
}
1852-
strcpy(p_args, buf);
1853-
1854-
/* Split string using space char */
1855-
token = strsep(&p_args, " ");
1856-
1857-
if (!token) {
1858-
dev_info(device,
1859-
"%s: error parsing args %d\n", __func__, -EINVAL);
1860-
goto free_m;
1861-
}
1862-
1863-
/* Convert 1st entry to int */
1864-
ret = kstrtoint (token, 10, &temp);
1842+
const char *p = buf;
1843+
char *endp;
1844+
1845+
temp = simple_strtoll(p, &endp, 10);
1846+
if (p == endp || *endp != ' ')
1847+
ret = -EINVAL;
1848+
else if (temp < INT_MIN || temp > INT_MAX)
1849+
ret = -ERANGE;
18651850
if (ret) {
18661851
dev_info(device,
18671852
"%s: error parsing args %d\n", __func__, ret);
1868-
goto free_m;
1853+
return size;
18691854
}
18701855

18711856
tl = int_to_short(temp);
18721857

1873-
/* Split string using space char */
1874-
token = strsep(&p_args, " ");
1875-
if (!token) {
1876-
dev_info(device,
1877-
"%s: error parsing args %d\n", __func__, -EINVAL);
1878-
goto free_m;
1879-
}
1880-
/* Convert 2nd entry to int */
1881-
ret = kstrtoint (token, 10, &temp);
1858+
p = endp + 1;
1859+
temp = simple_strtoll(p, &endp, 10);
1860+
if (p == endp)
1861+
ret = -EINVAL;
1862+
else if (temp < INT_MIN || temp > INT_MAX)
1863+
ret = -ERANGE;
18821864
if (ret) {
18831865
dev_info(device,
18841866
"%s: error parsing args %d\n", __func__, ret);
1885-
goto free_m;
1867+
return size;
18861868
}
18871869

18881870
/* Prepare to cast to short by eliminating out of range values */
@@ -1905,15 +1887,15 @@ static ssize_t alarms_store(struct device *device,
19051887
dev_info(device,
19061888
"%s: error reading from the slave device %d\n",
19071889
__func__, ret);
1908-
goto free_m;
1890+
return size;
19091891
}
19101892

19111893
/* Write data in the device RAM */
19121894
if (!SLAVE_SPECIFIC_FUNC(sl)) {
19131895
dev_info(device,
19141896
"%s: Device not supported by the driver %d\n",
19151897
__func__, -ENODEV);
1916-
goto free_m;
1898+
return size;
19171899
}
19181900

19191901
ret = SLAVE_SPECIFIC_FUNC(sl)->write_data(sl, new_config_register);
@@ -1922,10 +1904,6 @@ static ssize_t alarms_store(struct device *device,
19221904
"%s: error writing to the slave device %d\n",
19231905
__func__, ret);
19241906

1925-
free_m:
1926-
/* free allocated memory */
1927-
kfree(orig);
1928-
19291907
return size;
19301908
}
19311909

0 commit comments

Comments
 (0)