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

Re: [libvirt] [PATCH] logging: fix off-by-one bug



On Fri, Mar 18, 2011 at 08:31:22PM -0600, Eric Blake wrote:
> Valgrind caught that our log wrap-around was going 1 past the end.
> Regression introduced in commit b16f47a; previously the
> buffer was static and size+1 bytes, but now it is dynamic and
> exactly size bytes.
> 
> * src/util/logging.c (virLogStr): Don't write past end of log.
> ---
> 
> An alternative would be to malloc one larger; but since the log
> is likely to be a page size multiple and large enough to be worth
> malloc using mmap, going one larger is likely to waste the bulk
> of a page.  Also, I like always NUL-terminating the current end
> of the log (without including that NUL in the log length).
> 
>  src/util/logging.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/logging.c b/src/util/logging.c
> index b972f8a..f4910ad 100644
> --- a/src/util/logging.c
> +++ b/src/util/logging.c
> @@ -326,7 +326,7 @@ static void virLogStr(const char *str, int len) {
>          return;
>      if (len <= 0)
>          len = strlen(str);
> -    if (len > virLogSize)
> +    if (len >= virLogSize)
>          return;
>      virLogLock();
> 
> @@ -336,13 +336,13 @@ static void virLogStr(const char *str, int len) {
>      if (virLogEnd + len >= virLogSize) {
>          tmp = virLogSize - virLogEnd;
>          memcpy(&virLogBuffer[virLogEnd], str, tmp);
> -        virLogBuffer[virLogSize] = 0;
>          memcpy(&virLogBuffer[0], &str[tmp], len - tmp);
>          virLogEnd = len - tmp;
>      } else {
>          memcpy(&virLogBuffer[virLogEnd], str, len);
>          virLogEnd += len;
>      }
> +    virLogBuffer[virLogEnd] = 0;
>      /*
>       * Update the log length, and if full move the start index
>       */

 Well, I tend to prefer having a 0 at the end of character arrays
in C, this can be useful for example when debugging, so slightly
preferring one extra byte malloc for safety, either way not a big
deal, but let's fix this,

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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