Bug in netconsole?
I think there might be a bug in this commit:
The commit makes "the netconsole buffer size configurable". It adds CONFIG_NETCONSOLE_BUFFER_SIZE and maintains the original 512 default value used to define the length of input_buffer[]. nc_input_packet uses sizeof this to read packet data into input_buffer[]. This appears fine.
The commit also adds to following in the output chain:
@@ -214,7 +218,7 @@ static void nc_puts(const char *s)
len = strlen(s);
while (len) {
- int send_len = min(len, 512);
+ int send_len = min(len, sizeof(input_buffer));
nc_send_packet(s, send_len);
len -= send_len;
s += send_len;I can't see how this code relates to the sizeof input_buffer. The nc_puts data is written directly into NetTxPacket (plus header offsets) which is set to 1536 + alignment bytes long. If input_buffer is bigger than this, a buffer overflow will occur. Obviously the default value of 512 will not trigger the problem. The 512 magic number possibly ought to be derived from PKTSIZE_ALIGN (net.h), but I don't think sizeof(input_buffer) is appropriate here.
Regards,
Nick.
Hi Nick,
Quoted 18 lines · Nick Thompson
I think there might be a bug in this commit:
The commit makes "the netconsole buffer size configurable". It adds
CONFIG_NETCONSOLE_BUFFER_SIZEand maintains the original 512 default value used to define the length of input_buffer[].nc_input_packetuses sizeof this to read packet data into input_buffer[]. This appears fine.The commit also adds to following in the output chain:
(quoted diff) +1
-1@@ -214,7 +218,7 @@ static void nc_puts(const char *s) len = strlen(s); while (len) { - int send_len = min(len, 512); + int send_len = min(len, sizeof(input_buffer)); nc_send_packet(s, send_len); len -= send_len; s += send_len;
Quoted 1 line
I can't see how this code relates to the sizeof
input_buffer. Thenc_putsdata is written directly into NetTxPacket (plus header offsets) which is set to 1536 + alignment bytes long. Ifinput_bufferis bigger than this, a buffer overflow will occur. Obviously the default value of 512 will not trigger the problem. The 512 magic number possibly ought to be derived fromPKTSIZE_ALIGN(net.h), but I don't thinksizeof(input_buffer)is appropriate here.
I agree with your assessment based on this email (though I haven't dug into the code yet to fix it). Thanks for the report.
-Joe