[PATCH v2] firmware: tegra: bpmp: reject truncated debugfs entries

[PATCH v2] firmware: tegra: bpmp: reject truncated debugfs entries

[Pengpeng Hou]

The BPMP debugfs parser reads fixed-width values and bounded strings
from a size-delimited reply. Its fixed-width helper currently treats a
short copy as success, while the string helper advances past the range
when no NUL terminator is present. The status helper also uses an
unsigned return type for negative errors.

Require complete fixed-width reads and a terminator within the remaining
range before moving the cursor. Use a signed status result so errors
reach callers unchanged.

Fixes: f2381f652266 ("firmware: tegra: Add BPMP debugfs support")

Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1: https://lore.kernel.org/all/20260715083726.30740-1-pengpeng@iscas.ac.cn/
- no source-code changes
- rebase on the current Tegra firmware sources and tighten the commit message
- add the coding-assistant disclosure

The bounded parser helpers and callers were reviewed statically; no
malformed BPMP response was injected.

 drivers/firmware/tegra/bpmp-debugfs.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/tegra/bpmp-debugfs.c b/drivers/firmware/tegra/bpmp-debugfs.c
index 33c6300af964..bb2ce5be2e78 100644
--- a/drivers/firmware/tegra/bpmp-debugfs.c
+++ b/drivers/firmware/tegra/bpmp-debugfs.c
@@ -32,7 +32,7 @@ static size_t seqbuf_avail(struct seqbuf *seqbuf)
 	return seqbuf->pos < seqbuf->size ? seqbuf->size - seqbuf->pos : 0;
 }
 
-static size_t seqbuf_status(struct seqbuf *seqbuf)
+static int seqbuf_status(struct seqbuf *seqbuf)
 {
 	return seqbuf->pos <= seqbuf->size ? 0 : -EOVERFLOW;
 }
@@ -44,7 +44,9 @@ static int seqbuf_eof(struct seqbuf *seqbuf)
 
 static int seqbuf_read(struct seqbuf *seqbuf, void *buf, size_t nbyte)
 {
-	nbyte = min(nbyte, seqbuf_avail(seqbuf));
+	if (nbyte > seqbuf_avail(seqbuf))
+		return -EOVERFLOW;
+
 	memcpy(buf, seqbuf->buf + seqbuf->pos, nbyte);
 	seqbuf->pos += nbyte;
 	return seqbuf_status(seqbuf);
@@ -57,9 +59,16 @@ static int seqbuf_read_u32(struct seqbuf *seqbuf, u32 *v)
 
 static int seqbuf_read_str(struct seqbuf *seqbuf, const char **str)
 {
+	size_t avail;
+	size_t len;
+
+	avail = seqbuf_avail(seqbuf);
 	*str = seqbuf->buf + seqbuf->pos;
-	seqbuf->pos += strnlen(*str, seqbuf_avail(seqbuf));
-	seqbuf->pos++;
+	len = strnlen(*str, avail);
+	if (len == avail)
+		return -EOVERFLOW;
+
+	seqbuf->pos += len + 1;
 	return seqbuf_status(seqbuf);
 }
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] firmware: tegra: bpmp: reject truncated debugfs entries

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Fri, Aug 14, 2026 at 04:05:48PM +0800, Pengpeng Hou wrote:
> The BPMP debugfs parser reads fixed-width values and bounded strings
> from a size-delimited reply. Its fixed-width helper currently treats a
> short copy as success, while the string helper advances past the range
> when no NUL terminator is present. The status helper also uses an
> unsigned return type for negative errors.
> 
> Require complete fixed-width reads and a terminator within the remaining
> range before moving the cursor. Use a signed status result so errors
> reach callers unchanged.
> 
> Fixes: f2381f652266 ("firmware: tegra: Add BPMP debugfs support")
> 
> Assisted-by: Codex:gpt-5
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1: https://lore.kernel.org/all/20260715083726.30740-1-pengpeng@iscas.ac.cn/
> - no source-code changes
> - rebase on the current Tegra firmware sources and tighten the commit message
> - add the coding-assistant disclosure
> 
> The bounded parser helpers and callers were reviewed statically; no
> malformed BPMP response was injected.
> 
>  drivers/firmware/tegra/bpmp-debugfs.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)

This doesn't make sense to me. The seqbuf_read*() helpers already use
seqbuf_avail() to make sure they never read past the end of the buffer.
We also leave scope immediately anytime we see an overflow.

Worst case we'll see memcpy() copy 0 bytes, as far as I can tell, and
that's harmless (even though not entirely free).

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]