Re: [PATCH 1/2] tunables: Fix environment variable processing for setuid binaries

[Florian Weimer <fweimer at redhat dot com>]

On 01/29/2017 06:11 PM, Siddhesh Poyarekar wrote:
>  #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
> +# define ALLOC_SIZE 4096
> +/* Allocate bytes on heap to store tunable values copied over from the
> +   valstring.  We use a hardcoded ALLOC_SIZE to avoid querying the page size,
> +   since it may not be available this early in the startup process.  */
> +static char *
> +copy_to_heap (const char *in, size_t len)
> +{
> +  static size_t heap_size = 0;
> +  static char *heap = NULL;
> +  char *out;
> +
> +  if (heap_size < len + 1)
> +    {
> +      size_t ext = ALIGN_UP (len + 1, ALLOC_SIZE);

Is this really necessary?  That means that the tunable allocations are 
fairly substantial.

> +	      /* If we are in a secure context (AT_SECURE) then ignore the tunable
> +		 unless it is explicitly marked as secure.  Tunable values take
> +		 precendence over their envvar aliases.  */
> +	      if (__libc_enable_secure)
> +		{
> +		  if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)
> +		    {
> +		      char *q = &p[len];
> +		      while (*q != '\0')
> +			*name++ = *q++;
> +		      name[0] = '\0';
> +		      len = 0;
> +		    }
> +
> +		  if (cur->security_level != TUNABLE_SECLEVEL_NONE)
> +		    continue;
> +		}

This does not quite work because the next read position is not updated, 
although the tunable definition in the input string has been moved.  As 
a result, a subsequent tunable is not recognized and applied or filtered.

Thanks,
Florian