[Openais] [PATCH 1/8] COVERITY 12: prevent overrun of output buffers.

Steven Dake sdake at redhat.com
Fri Nov 20 17:08:36 PST 2009


On Sun, 2009-10-25 at 11:56 +1300, Angus Salkeld wrote:
> Overrun of static array "normal_output_buffer" of size 2048
> at position 2048 with index variable "normal_output_buffer_idx"
> 
> 536  		normal_output_buffer[normal_output_buffer_idx] = '\0';
> 537  		syslog_output_buffer[syslog_output_buffer_idx] = '\0';
> 538
> 
> Signed-off-by: Angus Salkeld <angus.salkeld at gmail.com>
> ---
>   exec/logsys.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/exec/logsys.c b/exec/logsys.c
> index 978c157..395a59d 100644
> --- a/exec/logsys.c
> +++ b/exec/logsys.c
> @@ -527,8 +527,8 @@ static void log_printf_to_logs (
>   			syslog_output_buffer_idx += syslog_len;
>   			format_buffer_idx += 1;
>   		}
> -		if ((normal_output_buffer_idx == sizeof (normal_output_buffer)) ||
> -		    (syslog_output_buffer_idx == sizeof (syslog_output_buffer))) {
> +		if ((normal_output_buffer_idx >= sizeof (normal_output_buffer - 2)) ||
> +		    (syslog_output_buffer_idx >= sizeof (syslog_output_buffer - 1))) {
>   			break;
>   		}
>   	}

this code doesn't make any sense.  the code takes the sizeof a buffer-2
(which if i parse with my eyes is sizeof (&buffer[-2])).  instead it
should be something like sizeof (normal_output_buffer) - 1.

This raises a further question why use - 2 in one stanza and -1 in
another?  Wouldn't they both be -1?

Regards
-steve




More information about the Openais mailing list