On Fri, 2009-01-09 at 13:47 -0500, Valerie Aurora Henson wrote:

Just a first pass reading but noticed ....

> diff --git a/lib/cache.c b/lib/cache.c
> index 36b8294..bc2c722 100644
> --- a/lib/cache.c
> +++ b/lib/cache.c
> @@ -484,27 +484,19 @@ struct mapent *cache_lookup_offset(const char *prefix, 
> const char *offset, int s
>  {
>       struct list_head *p;
>       struct mapent *this;
> -     int plen = strlen(prefix);
> -     char *o_key;
> +     char o_key[KEY_MAX_LEN];

I know this looks right but I think it will end up being a problem
latter. I know there are places in the map lookup libraries that
restrict these to KEY_MAX_LEN, which is needed for indirect map keys,
but direct map keys really shouldn't have this restriction as they
should be able to be as long as PATH_MAX but that brings the issue of
needing to audit and adjust the maximum parse buffer as well. So we will
need to get around to that at some point as well.

>  
>       /* root offset duplicates "/" */
> -     if (plen > 1) {
> -             o_key = alloca(plen + strlen(offset) + 1);
> -             strcpy(o_key, prefix);
> -             strcat(o_key, offset);
> -     } else {
> -             o_key = alloca(strlen(offset) + 1);
> -             strcpy(o_key, offset);
> -     }
> +     if (snprintf(o_key, sizeof(o_key), "%s%s", prefix, offset) >
> +         sizeof(o_key))
> +             return NULL;

Looks like this will set o_key to "//..." for strlen(prefix) == 1.

>  
>       list_for_each(p, head) {
>               this = list_entry(p, struct mapent, multi_list);
>               if (!strcmp(&this->key[start], o_key))
> -                     goto done;
> +                     return this;
>       }
> -     this = NULL;
> -done:
> -     return this;
> +     return NULL;
>  }
>  


_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Reply via email to