Thread (10 messages) 10 messages, 3 authors, 2025-09-15

Re: [PATCH net-next v6 3/5] dpll: zl3073x: Add firmware loading functionality

From: Ivan Vecera <ivecera@redhat.com>
Date: 2025-09-15 08:18:23
Also in: linux-doc, lkml


On 14. 09. 25 11:45 odp., Jakub Kicinski wrote:
On Tue,  9 Sep 2025 11:15:30 +0200 Ivan Vecera wrote:
quoted
+	/* Fetch image name and size from input */
+	strscpy(buf, *psrc, min(sizeof(buf), *psize));
+	rc = sscanf(buf, "%15s %u %n", name, &count, &pos);
+	if (!rc) {
+		/* No more data */
+		return 0;
+	} else if (rc == 1 || count > U32_MAX / sizeof(u32)) {
+		ZL3073X_FW_ERR_MSG(extack, "invalid component size");
+		return -EINVAL;
+	}
+	*psrc += pos;
+	*psize -= pos;
Still worried about pos not being bounds checked.
Admin can crash the kernel with invalid FW file.

	if (pos > *psize)
		/* error */
This cannot happen...

1) strscpy(buf, *psrc, min(sizeof(buf, *psize)) ensures that the string
    will be zero padded and strlen(buf) will be less than *psize

2) sscanf(buf, "%15s %u %n", name, &count, &pos) scans for string with
    max length of 15, one or more whitespace(s), number and one or more
    whitespaces(s). And reports number of parsed arguments. Note that
    the %n does not increase the count returned.

So... if:
1) buf is empty then sscanf returns 0 and /* No more data */ code path
    is executed
2) buf contains only string (1st argument) OR string and non-numeric
    2nd argument then the sscanf returns 1 and 'invalid component size'
    is executed
3) buf contains string (1st arg) and numeric 2nd arg then the sscanf
    returns 2 and the 2nd arg is stored into 'count' and number of
    consumed characters into 'pos'
4) if 'count' > (U32_MAX / 4) then 'invalid component size' path is
    executed
Also what if sscanf() return 2? pos is uninitialized?
If the sscanf() returns 2 then pos is initialized, in other words
'pos' is initialized and less then *psize if the 'count' is correctly
parsed as the numeric value.

Thanks,
Ivan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help