Re: [PATCH net 1/3] netconsole: return count instead of strnlen(buf, count) from store callbacks

[Breno Leitao <leitao@debian.org>]

Hello Simon,

On Sun, Apr 26, 2026 at 09:46:40AM +0100, Simon Horman wrote:
> On Thu, Apr 23, 2026 at 02:41:15AM -0700, Breno Leitao wrote:
> > Several configfs store callbacks in netconsole end with:
> >
> > 	ret = strnlen(buf, count);
> >
> > This under-reports the number of bytes consumed when the input
> > contains an embedded NUL within count, telling the VFS that fewer
> > bytes were written than userspace actually handed in. A conformant
> > partial-write loop would then retry the trailing bytes against a
> > callback that has already accepted them.
> >
> > Every other configfs driver in the tree returns count directly from
> > its store callbacks once parsing has succeeded, including
> > drivers/nvme/target/configfs.c, drivers/gpio/gpio-sim.c,
> > drivers/most/configfs.c, drivers/block/null_blk/main.c,
> > drivers/pci/endpoint/pci-ep-cfs.c, and the rest of the configfs
> > users. netconsole was the outlier (along with
> > drivers/infiniband/core/cma_configfs.c, which has the same latent
> > issue).
> >
> > Align netconsole with the rest of the configfs ecosystem: return
> > count once the parser/validator has accepted the input. The numeric
> > and boolean parsers (kstrtobool, kstrtou16, mac_pton,
> > netpoll_parse_ip_addr) have already validated the meaningful prefix;
> > any trailing bytes are padding and should simply be reported as
> > consumed.
> >
> > Fixes: 0bcc1816188e ("[NET] netconsole: Support dynamic reconfiguration using configfs")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> FTR: Sashiko has provided an AI generated review of this patch.
> Like it's review of patch 3/3 - which I forwarded separately -
> it flags that trim_newline() may perform an OOB access
> if passed an empty string. But this is not correct because
> trim_newline() correctly handles this case.

You're absolutely right. This has been addressed in the patch that recently
landed in Linus' tree.

It appears Sashiko ran its analysis on a tree predating that fix.

https://github.com/torvalds/linux/commit/7079c8c13f2d33992bc846240517d88f4ab07781

Thanks for the review,
--breno