Re: [PATCH net] netconsole: avoid out-of-bounds access on empty string in trim_newline()

[Breno Leitao]

On Tue, Apr 21, 2026 at 05:22:19PM +0100, Simon Horman wrote:
> On Mon, Apr 20, 2026 at 03:18:36AM -0700, Breno Leitao wrote:
> > trim_newline() unconditionally dereferences s[len - 1] after computing
> > len = strnlen(s, maxlen). When the string is empty, len is 0 and the
> > expression underflows to s[(size_t)-1], reading (and potentially
> > writing) one byte before the buffer.
> > 
> > The two callers feed trim_newline() with the result of strscpy() from
> > configfs store callbacks (dev_name_store, userdatum_value_store).
> > configfs guarantees count >= 1 reaches the callback, but the byte
> > itself can be NUL: a userspace write(fd, "\0", 1) leaves the
> > destination empty after strscpy() and triggers the underflow. The OOB
> > write only fires if the adjacent byte happens to be '\n', so this is
> > not a security issue, but the access is undefined behaviour either way.
> > 
> > This pattern is commonly flagged by LLM-based code reviewers. While it
> > is not a security fix, the underlying access is undefined behaviour and
> > the change is small and self-contained, so it is a reasonable candidate
> > for the stable trees.
> > 
> > Guard the dereference on a non-zero length.
> > 
> > Fixes: ae001dc67907 ("net: netconsole: move newline trimming to function")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> 
> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> 
> Sashiko has provided some feedback on this patch.
> I do not believe that should hold up progress of this patch.
> But I'd appreciate it if you could look over that feedback
> and see if any follow-up is warranted.

Thanks for the review, I've had a quick look, and it is complaining
about problems are not regressions, but some other issues in the code,
which I will need to check more carefully tomorrow.

https://sashiko.dev/#/patchset/20260420-netcons_trim_newline-v1-1-dc35889aeedf%40debian.org

Thanks,
--breno