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: BZ# 16418: Fix powerpc get_clockfreq raciness


Ping.

On 24-11-2014 19:44, Adhemerval Zanella wrote:
> This patch fixes powerpc __get_clockfreq racy and cancel-safe issues by
> dropping internal static cache and by using nocancel file operations.
> The vDSO failure check is also removed, since kernel code does not
> return an error (it cleans cr0.so bit on function return) and the static
> code (to read value /proc) now uses non-cancellable calls.
>
> Since currently I don't see this code patch to be performance sensitive
> (usually the clock frequency is obtained once to transform timebase
> values), I don't see a problem to drop its internal cache.  Also, if
> latency came up as being important for this one, correct approach would
> be use IFUNC to call vDSO symbols directly (which I do not aim to
> implement now).
>
> Tested on powerpc64 and powerpc32.
>
> --
>
> 	[BZ# 16418]
> 	* sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
> 	(__get_clockfreq): Make code racy and cancel safe.
>
> ---
>
> diff --git a/NEWS b/NEWS
> index ad170c4..833a680 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,10 +9,10 @@ Version 2.21
>
>  * The following bugs are resolved with this release:
>
> -  6652, 12926, 14132, 14138, 14171, 14498, 15215, 15884, 16469, 17266,
> -  17344, 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506,
> -  17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583,
> -  17584, 17585, 17589, 17594, 17616, 17625.
> +  6652, 12926, 14132, 14138, 14171, 14498, 15215, 15884, 16418, 16469,
> +  17266, 17344, 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501,
> +  17506, 17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582,
> +  17583, 17584, 17585, 17589, 17594, 17616, 17625.
>
>  * CVE-2104-7817 The wordexp function could ignore the WRDE_NOCMD flag
>    under certain input conditions resulting in the execution of a shell for
> diff --git a/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c b/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
> index 62217b1..44f90b4 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/get_clockfreq.c
> @@ -24,95 +24,85 @@
>  #include <libc-internal.h>
>  #include <sysdep.h>
>  #include <bits/libc-vdso.h>
> +#include <not-cancel.h>
>
>  hp_timing_t
>  __get_clockfreq (void)
>  {
> +  hp_timing_t result = 0L;
> +
> +#ifdef SHARED
> +  /* The vDSO does not return an error (it clear cr0.so on returning).  */
> +  INTERNAL_SYSCALL_DECL (err);
> +  result =
> +    INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);
> +#else
>    /* We read the information from the /proc filesystem.  /proc/cpuinfo
>       contains at least one line like:
>       timebase        : 33333333
>       We search for this line and convert the number into an integer.  */
> -  static hp_timing_t timebase_freq;
> -  hp_timing_t result = 0L;
> +  int fd = __open_nocancel ("/proc/cpuinfo", O_RDONLY);
> +  if (__glibc_likely (fd != -1))
> +    return result;
>
> -  /* If this function was called before, we know the result.  */
> -  if (timebase_freq != 0)
> -    return timebase_freq;
> +  /* The timebase will be in the 1st 1024 bytes for systems with up
> +     to 8 processors.  If the first read returns less then 1024
> +     bytes read,  we have the whole cpuinfo and can start the scan.
> +     Otherwise we will have to read more to insure we have the
> +     timebase value in the scan.  */
> +  char buf[1024];
> +  ssize_t n;
>
> -  /* If we can use the vDSO to obtain the timebase even better.  */
> -#ifdef SHARED
> -  INTERNAL_SYSCALL_DECL (err);
> -  timebase_freq =
> -    INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);
> -  if (INTERNAL_SYSCALL_ERROR_P (timebase_freq, err)
> -      && INTERNAL_SYSCALL_ERRNO (timebase_freq, err) == ENOSYS)
> -#endif
> +  n = __read_nocancel (fd, buf, sizeof (buf));
> +  if (n == sizeof (buf))
>      {
> -      int fd = __open ("/proc/cpuinfo", O_RDONLY);
> +      /* We are here because the 1st read returned exactly sizeof
> +         (buf) bytes.  This implies that we are not at EOF and may
> +         not have read the timebase value yet.  So we need to read
> +         more bytes until we know we have EOF.  We copy the lower
> +         half of buf to the upper half and read sizeof (buf)/2
> +         bytes into the lower half of buf and repeat until we
> +         reach EOF.  We can assume that the timebase will be in
> +         the last 512 bytes of cpuinfo, so two 512 byte half_bufs
> +         will be sufficient to contain the timebase and will
> +         handle the case where the timebase spans the half_buf
> +         boundry.  */
> +      const ssize_t half_buf = sizeof (buf) / 2;
> +      while (n >= half_buf)
> +	{
> +	  memcpy (buf, buf + half_buf, half_buf);
> +	  n = __read_nocancel (fd, buf + half_buf, half_buf);
> +	}
> +      if (n >= 0)
> +	n += half_buf;
> +    }
> +  __close_nocancel (fd);
>
> -      if (__glibc_likely (fd != -1))
> +  if (__glibc_likely (n > 0))
> +    {
> +      char *mhz = memmem (buf, n, "timebase", 7);
> +
> +      if (__glibc_likely (mhz != NULL))
>  	{
> -	  /* The timebase will be in the 1st 1024 bytes for systems with up
> -	     to 8 processors.  If the first read returns less then 1024
> -	     bytes read,  we have the whole cpuinfo and can start the scan.
> -	     Otherwise we will have to read more to insure we have the
> -	     timebase value in the scan.  */
> -	  char buf[1024];
> -	  ssize_t n;
> +	  char *endp = buf + n;
>
> -	  n = __read (fd, buf, sizeof (buf));
> -	  if (n == sizeof (buf))
> -	    {
> -	      /* We are here because the 1st read returned exactly sizeof
> -	         (buf) bytes.  This implies that we are not at EOF and may
> -	         not have read the timebase value yet.  So we need to read
> -	         more bytes until we know we have EOF.  We copy the lower
> -	         half of buf to the upper half and read sizeof (buf)/2
> -	         bytes into the lower half of buf and repeat until we
> -	         reach EOF.  We can assume that the timebase will be in
> -	         the last 512 bytes of cpuinfo, so two 512 byte half_bufs
> -	         will be sufficient to contain the timebase and will
> -	         handle the case where the timebase spans the half_buf
> -	         boundry.  */
> -	      const ssize_t half_buf = sizeof (buf) / 2;
> -	      while (n >= half_buf)
> -		{
> -		  memcpy (buf, buf + half_buf, half_buf);
> -		  n = __read (fd, buf + half_buf, half_buf);
> -		}
> -	      if (n >= 0)
> -		n += half_buf;
> -	    }
> +	  /* Search for the beginning of the string.  */
> +	  while (mhz < endp && (*mhz < '0' || *mhz > '9') && *mhz != '\n')
> +	    ++mhz;
>
> -	  if (__builtin_expect (n, 1) > 0)
> +	  while (mhz < endp && *mhz != '\n')
>  	    {
> -	      char *mhz = memmem (buf, n, "timebase", 7);
> -
> -	      if (__glibc_likely (mhz != NULL))
> +	      if (*mhz >= '0' && *mhz <= '9')
>  		{
> -		  char *endp = buf + n;
> -
> -		  /* Search for the beginning of the string.  */
> -		  while (mhz < endp && (*mhz < '0' || *mhz > '9')
> -			 && *mhz != '\n')
> -		    ++mhz;
> -
> -		  while (mhz < endp && *mhz != '\n')
> -		    {
> -		      if (*mhz >= '0' && *mhz <= '9')
> -			{
> -			  result *= 10;
> -			  result += *mhz - '0';
> -			}
> -
> -		      ++mhz;
> -		    }
> +		  result *= 10;
> +		  result += *mhz - '0';
>  		}
> -	      timebase_freq = result;
> +
> +	      ++mhz;
>  	    }
> -	  __close (fd);
>  	}
>      }
> +#endif
>
> -  return timebase_freq;
> +  return result;
>  }
>


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