[lxc-devel] [PATCH] hash command unix socket name

[S.Çağlar Onur]

On Wed, Feb 19, 2014 at 4:46 PM, Stéphane Graber <stgraber at ubuntu.com> wrote:
> On Wed, Feb 19, 2014 at 04:31:38PM -0500, S.Çağlar Onur wrote:
>> Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
>> ---
>>  src/lxc/commands.c | 33 +++++++++++++++++++++++++++------
>>  1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/lxc/commands.c b/src/lxc/commands.c
>> index 6b46c2c..dbfa23e 100644
>> --- a/src/lxc/commands.c
>> +++ b/src/lxc/commands.c
>> @@ -32,6 +32,8 @@
>>  #include <sys/param.h>
>>  #include <malloc.h>
>>  #include <stdlib.h>
>> +#include <stdbool.h>
>> +#include <inttypes.h>
>>
>>  #include "log.h"
>>  #include "lxc.h"
>> @@ -74,10 +76,12 @@
>>  lxc_log_define(lxc_commands, lxc);
>>
>>  static int fill_sock_name(char *path, int len, const char *name,
>> -                       const char *inpath)
>> +                       const char *inpath, bool init)
>>  {
>> +     char *offset = &path[1];
>>       const char *lxcpath = NULL;
>> -     int ret;
>> +     uint64_t hash;
>> +     int ret, sock;
>>
>>       if (!inpath) {
>>               lxcpath = lxc_global_config_value("lxc.lxcpath");
>> @@ -86,12 +90,29 @@ static int fill_sock_name(char *path, int len, const char *name,
>>                       return -1;
>>               }
>>       }
>> -     ret = snprintf(path, len, "%s/%s/command", lxcpath ? lxcpath : inpath, name);
>> -
>> +     ret = snprintf(offset, len, "%s/%s/command", lxcpath ? lxcpath : inpath, name);
>>       if (ret < 0 || ret >= len) {
>>               ERROR("Name too long");
>>               return -1;
>>       }
>
> ^ So wasn't the whole point of this change to allow for a lxcpath/name
> to be bigger than 108 bytes?
>
> If so, the above check will prevent that from working as the path itself
> is still constrained by the length passed from the caller.
>
>> +     /* addr.sun_path is only 108 bytes, so we hash the full name */
>> +     hash = fnv_64a_buf(path, ret, FNV1A_64_INIT);
>
> So we're hashing the whole thing, not just offset?

Yep you are right in both cases. New version is coming...

>> +     ret = snprintf(offset, len, "lxc/command/%016" PRIx64, hash);
>> +     if (ret < 0 || ret >= len)
>> +             return -1;
>> +
>> +     /* if caller is not lxc_cmd_init then try connecting to UDS,
>> +      * fallback to the old naming scheme if that fails */
>> +     if (!init) {
>> +             sock = lxc_abstract_unix_connect(path);
>> +             if (sock < 0 && errno == ECONNREFUSED) {
>> +                     ret = snprintf(offset, len, "%s/%s/command", lxcpath ? lxcpath : inpath, name);
>> +                     if (ret < 0 || ret >= len) {
>> +                             return -1;
>> +                     }
>> +             }
>> +             close(sock);
>> +     }
>>       return 0;
>>  }
>>
>> @@ -248,7 +269,7 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
>>       *stopped = 0;
>>
>>       len = sizeof(path)-1;
>> -     if (fill_sock_name(offset, len, name, lxcpath))
>> +     if (fill_sock_name(path, len, name, lxcpath, false))
>>               return -1;
>>
>>       sock = lxc_abstract_unix_connect(path);
>> @@ -856,7 +877,7 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler,
>>       int len;
>>
>>       len = sizeof(path)-1;
>> -     if (fill_sock_name(offset, len, name, lxcpath))
>> +     if (fill_sock_name(path, len, name, lxcpath, true))
>>               return -1;
>>
>>       fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0);
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> lxc-devel mailing list
>> lxc-devel at lists.linuxcontainers.org
>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>
> --
> Stéphane Graber
> Ubuntu developer
> http://www.ubuntu.com
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
>



-- 
S.Çağlar Onur <caglar at 10ur.org>