On Fri, Nov 7, 2025 at 12:15 PM Breno Leitao <[email protected]> wrote: > > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c > > index 5d8d0214786c..0a8ba7c4bc9d 100644 > > --- a/drivers/net/netconsole.c > > +++ b/drivers/net/netconsole.c > > @@ -1553,13 +1553,16 @@ static void send_fragmented_body(struct > > netconsole_target *nt, > > const char *msgbody, int header_len, > > int msgbody_len, int extradata_len) > > { > > - int sent_extradata, preceding_bytes; > > const char *extradata = NULL; > > int body_len, offset = 0; > > + int extradata_offset = 0; > > + int msgbody_offset = 0; > > > > #ifdef CONFIG_NETCONSOLE_DYNAMIC > > extradata = nt->extradata_complete; > > #endif > > > extradata could be NULL at this time if CONFIG_NETCONSOLE_DYNAMIC is > unset. Basically extradata=NULL will not be replaced. > > > + if (WARN_ON_ONCE(!extradata && extradata_len != 0)) > > + return; > > And entradata_len = 0 for CONFIG_NETCONSOLE_DYNAMIC disabled. > > > + /* write msgbody first */ > > + this_chunk = min(msgbody_len - msgbody_offset, > > + MAX_PRINT_CHUNK - this_header); > > + memcpy(nt->buf + this_header, msgbody + msgbody_offset, > > + this_chunk); > > + msgbody_offset += this_chunk; > > + this_offset += this_chunk; > > + > > + /* after msgbody, append extradata */ > > + this_chunk = min(extradata_len - extradata_offset, > > + MAX_PRINT_CHUNK - this_header - this_offset); > > + memcpy(nt->buf + this_header + this_offset, > > + extradata + extradata_offset, this_chunk); > > then you are going to memcpy from NULL pointer (`extradata + > extradata_offset` == 0).
I believe passing NULL to memcpy() should be safe as long as count is zero (which is the case here). However, what I didn't realize at first is that we would be doing pointer arithmetic with NULL, which is undefined behavior :( I will add a check if extradata is NULL here. Thanks for the careful review! > > I got this my vim LSP that printed: > > Null pointer passed as 2nd argument to memory copy function > [unix.cstring.NullArg] > On Fri, Nov 7, 2025 at 12:15 PM Breno Leitao <[email protected]> wrote: > > On Wed, Nov 05, 2025 at 09:06:43AM -0800, Gustavo Luiz Duarte wrote: > > Refactor send_fragmented_body() to use separate offset tracking for > > msgbody, and extradata instead of complex conditional logic. > > The previous implementation used boolean flags and calculated offsets > > which made the code harder to follow. > > > > The new implementation maintains independent offset counters > > (msgbody_offset, extradata_offset) and processes each section > > sequentially, making the data flow more straightforward and the code > > easier to maintain. > > > > This is a preparatory refactoring with no functional changes, which will > > allow easily splitting extradata_complete into separate userdata and > > sysdata buffers in the next patch. > > > > Signed-off-by: Gustavo Luiz Duarte <[email protected]> > > --- > > drivers/net/netconsole.c | 73 > > ++++++++++++++++-------------------------------- > > 1 file changed, 24 insertions(+), 49 deletions(-) > > > > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c > > index 5d8d0214786c..0a8ba7c4bc9d 100644 > > --- a/drivers/net/netconsole.c > > +++ b/drivers/net/netconsole.c > > @@ -1553,13 +1553,16 @@ static void send_fragmented_body(struct > > netconsole_target *nt, > > const char *msgbody, int header_len, > > int msgbody_len, int extradata_len) > > { > > - int sent_extradata, preceding_bytes; > > const char *extradata = NULL; > > int body_len, offset = 0; > > + int extradata_offset = 0; > > + int msgbody_offset = 0; > > > > #ifdef CONFIG_NETCONSOLE_DYNAMIC > > extradata = nt->extradata_complete; > > #endif > > > extradata could be NULL at this time if CONFIG_NETCONSOLE_DYNAMIC is > unset. Basically extradata=NULL will not be replaced. > > > + if (WARN_ON_ONCE(!extradata && extradata_len != 0)) > > + return; > > And entradata_len = 0 for CONFIG_NETCONSOLE_DYNAMIC disabled. > > > + /* write msgbody first */ > > + this_chunk = min(msgbody_len - msgbody_offset, > > + MAX_PRINT_CHUNK - this_header); > > + memcpy(nt->buf + this_header, msgbody + msgbody_offset, > > + this_chunk); > > + msgbody_offset += this_chunk; > > + this_offset += this_chunk; > > + > > + /* after msgbody, append extradata */ > > + this_chunk = min(extradata_len - extradata_offset, > > + MAX_PRINT_CHUNK - this_header - this_offset); > > + memcpy(nt->buf + this_header + this_offset, > > + extradata + extradata_offset, this_chunk); > > then you are going to memcpy from NULL pointer (`extradata + > extradata_offset` == 0). > > I got this my vim LSP that printed: > > Null pointer passed as 2nd argument to memory copy function > [unix.cstring.NullArg] >
