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

Steven Dake sdake at redhat.com
Sun Nov 22 14:06:03 PST 2009


good for merge

regards
-steve

On Mon, 2009-11-23 at 08:09 +1300, angus salkeld wrote:
> On Fri, 2009-11-20 at 18:08 -0700, Steven Dake wrote:
> > 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
> > 
> > 
> 
> Well spotted, I "just" added the " - 1" in the wrong place - oops.
> 
> Here is a fixed patch (with a comment).
> 
> -Angus
> 
> ===================================================================
> --- exec/logsys.c	(revision 2546)
> +++ exec/logsys.c	(working copy)
> @@ -527,8 +527,12 @@
>  			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)) {
> +			/* Note: we make allowance for '\0' at the end of
> +			 * both of these arrays and normal_output_buffer also
> +			 * needs a '\n'.
> +			 */
>  			break;
>  		}
>  	}
> 
> 
> 
> _______________________________________________
> Openais mailing list
> Openais at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/openais



More information about the Openais mailing list