[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