This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: bug in times.c


  Hi!

On Mon, Mar 11, 2013 at 11:30:19AM +0100, Holger Brunck wrote:
> Here is the diff to the current glibc git tree for the proposed solution:
> 
> diff --git a/sysdeps/unix/sysv/linux/times.c b/sysdeps/unix/sysv/linux/times.c
> index f3b5f01..ba20e5b 100644
> --- a/sysdeps/unix/sysv/linux/times.c
> +++ b/sysdeps/unix/sysv/linux/times.c
> @@ -39,11 +39,12 @@ __times (struct tms *buf)
>         asm volatile ("" : "+r" (temp));                                      \
>         v = temp;                                                             \
>        } while (0)
> -      touch (buf->tms_utime);
> -      touch (buf->tms_stime);
> -      touch (buf->tms_cutime);
> -      touch (buf->tms_cstime);
> -
> +      if (buf != NULL) {
> +        touch (buf->tms_utime);
> +        touch (buf->tms_stime);
> +        touch (buf->tms_cutime);
> +        touch (buf->tms_cstime);
> +      }
>        /* If we come here the memory is valid and the kernel did not
>          return an EFAULT error.  Return the value given by the kernel.  */
>      }
> 
> Any comments on this? should I send a regular git patch?

  I think a slightly more elegant version would be

2013-03-11  Petr Baudis  <pasky@ucw.cz>

	* sysdeps/unix/sysv/linux/times.c (__times): On EFAULT, test for
	non-NULL pointer before the memory validity test. Pointed out
	by Holger Brunck <holger.brunck@keymile.com>.

diff --git a/sysdeps/unix/sysv/linux/times.c b/sysdeps/unix/sysv/linux/times.c
index f3b5f01..200fc2d 100644
--- a/sysdeps/unix/sysv/linux/times.c
+++ b/sysdeps/unix/sysv/linux/times.c
@@ -26,7 +26,8 @@ __times (struct tms *buf)
   INTERNAL_SYSCALL_DECL (err);
   clock_t ret = INTERNAL_SYSCALL (times, err, 1, buf);
   if (INTERNAL_SYSCALL_ERROR_P (ret, err)
-      && __builtin_expect (INTERNAL_SYSCALL_ERRNO (ret, err) == EFAULT, 0))
+      && __builtin_expect (INTERNAL_SYSCALL_ERRNO (ret, err) == EFAULT, 0)
+      && buf != NULL)
     {
       /* This might be an error or not.  For architectures which have
 	 no separate return value and error indicators we cannot
@@ -44,7 +45,8 @@ __times (struct tms *buf)
       touch (buf->tms_cutime);
       touch (buf->tms_cstime);
 
-      /* If we come here the memory is valid and the kernel did not
+      /* If we come here the memory is valid (or BUF is NULL, which is
+       * a valid condition for the kernel syscall) and the kernel did not
 	 return an EFAULT error.  Return the value given by the kernel.  */
     }
 

  (Unfortunately, it's untested as I currently don't have a glibc build
infrastructure except for locale building. Ok to commit still?)

-- 
				Petr "Pasky" Baudis
	For every complex problem there is an answer that is clear,
	simple, and wrong.  -- H. L. Mencken


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]