Re: [PATCH 1/2] usb: host: uhci-debug: use scnprintf() instead of sprintf()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/13/22 1:33 AM, David Laight wrote:

[...]
>> The UHCI driver's debugging code uses a lot of sprintf() calls with the
>> large buffers, leaving some space at the end of the buffers to handle the
>> buffer overflow. Using scnprntf() instead eliminates the very possibility
>> of the buffer overflow, while simplifying the code at the expense of not
>> printing an ellipsis when the end of buffer is actually reached...
> 
> Hmmm...
> 
> The old code seems to so:

   s/so/do/? :-)

>> -	out += sprintf(out, "(buf=%08x)\n", hc32_to_cpu(uhci, td->buffer));
>>
>> -	if (out - buf > len)
>> -		out += sprintf(out, " ...\n");
> 
> Which is going to overflow the output buffer unless there
> is enough 'tailroom' after buf[len] for all the sprintf()

   There are 1024 bytes (EXTRA_SPACE)...

> before any length check and the ellipsis.
> 
> The new code won't overrun buf[len] but also fails to
> '\n' terminate long lines.

   Yes.
   And one also has problems correctly identifying the overflowing lines (iff
such line ends exactly at end of buffer)... :-(

> So you probably do need a check for:
> 	if (out == len - 1 && buf[out - 1] != '\n')

   'out' is a pointer, you probably meant:

	if (out - buf == len - 1 && *(out - 1) != '\n')

> 		strcpy(buf + len - 5, "...\n");

   That's not exactly what's done by the existing code... I think we'd be
better off using strrchr()... but then again, we're not sure we have at least
5 bytes...

> 	David

[...]

MBR, Sergey



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux