[Patch] Avoid duplicate names in /proc/registry (which may crash find)

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Here is a simple approach to handle the duplicate key/value name problem
in /proc/registry. A value is skipped if key with same name exists.
Number of actual key existence checks are reduced by a simple hash table.

The patch also adds dirent.d_type support, find does no longer crash.

Christian


2008-12-04  Christian Franke  <[hidden email]>

        * fhandler_registry.cc (__DIR_hash): New class.
        (d_hash): New macro.
        (key_exists): New function.
        (fhandler_registry::readdir): Allocate __DIR_hash.
        Record key names in hash table. Skip value if key
        with same name exists. Fix error handling of
        encode_regname (). Set dirent.d_type.
        (fhandler_registry::closedir): Delete __DIR_hash.



diff --git a/winsup/cygwin/fhandler_registry.cc b/winsup/cygwin/fhandler_registry.cc
index ce4335f..4c95c77 100644
--- a/winsup/cygwin/fhandler_registry.cc
+++ b/winsup/cygwin/fhandler_registry.cc
@@ -143,6 +143,48 @@ decode_regname (char * dst, const char * src, int len = -1)
   return 0;
 }
 
+
+/* Hash table to limit calls to key_exists ().
+ */
+class __DIR_hash
+{
+public:
+  __DIR_hash ()
+    {
+      memset (table, 0, sizeof(table));
+    }
+
+  void set (unsigned h)
+    {
+      table [(h >> 3) & (HASH_SIZE - 1)] |= (1 << (h & 0x3));
+    }
+
+  bool is_set (unsigned h) const
+    {
+      return (table [(h >> 3) & (HASH_SIZE - 1)] & (1 << (h & 0x3))) != 0;
+    }
+
+private:
+  enum { HASH_SIZE = 1024 };
+  unsigned char table[HASH_SIZE];
+};
+
+#define d_hash(d) ((__DIR_hash *) (d)->__d_internal)
+
+
+/* Return true if subkey NAME exists in key PARENT.
+ */
+static bool
+key_exists (HKEY parent, const char * name, DWORD wow64)
+{
+  HKEY hKey = (HKEY) INVALID_HANDLE_VALUE;
+  LONG error = RegOpenKeyEx (parent, name, 0, KEY_READ | wow64, &hKey);
+  if (error == ERROR_SUCCESS)
+    RegCloseKey (hKey);
+
+  return (error == ERROR_SUCCESS || error == ERROR_ACCESS_DENIED);
+}
+
 /* Returns 0 if path doesn't exist, >0 if path is a directory,
  * <0 if path is a file.
  *
@@ -381,13 +423,16 @@ fhandler_registry::readdir (DIR *dir, dirent *de)
       res = 0;
       goto out;
     }
-  if (dir->__handle == INVALID_HANDLE_VALUE && dir->__d_position == 0)
+  if (dir->__handle == INVALID_HANDLE_VALUE)
     {
+      if (dir->__d_position != 0)
+ goto out;
       handle = open_key (path + 1, KEY_READ, wow64, false);
       dir->__handle = handle;
+      if (dir->__handle == INVALID_HANDLE_VALUE)
+ goto out;
+      dir->__d_internal = (unsigned) new __DIR_hash ();
     }
-  if (dir->__handle == INVALID_HANDLE_VALUE)
-    goto out;
   if (dir->__d_position < SPECIAL_DOT_FILE_COUNT)
     {
       strcpy (de->d_name, special_dot_files[dir->__d_position++]);
@@ -425,14 +470,37 @@ retry:
     }
 
   /* We get here if `buf' contains valid data.  */
+  dir->__d_position++;
+  if (dir->__d_position & REG_ENUM_VALUES_MASK)
+    dir->__d_position += 0x10000;
+
   if (*buf == 0)
     strcpy (de->d_name, DEFAULT_VALUE_NAME);
-  else if (encode_regname (de->d_name, buf))
-    goto retry;
+  else
+    {
+      /* Skip value if name is identical to a previous key name.  */
+      unsigned h = hash_path_name (1, buf);
+      if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
+ d_hash (dir)->set (h);
+      else if (d_hash (dir)->is_set (h)
+       && key_exists ((HKEY) dir->__handle, buf, wow64))
+ {
+  buf_size = NAME_MAX + 1;
+  goto retry;
+ }
+
+      if (encode_regname (de->d_name, buf))
+ {
+  buf_size = NAME_MAX + 1;
+  goto retry;
+ }
+    }
 
-  dir->__d_position++;
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
-    dir->__d_position += 0x10000;
+    de->d_type = DT_REG;
+  else
+    de->d_type = DT_DIR;
+
   res = 0;
 out:
   syscall_printf ("%d = readdir (%p, %p)", res, dir, de);
@@ -473,11 +541,14 @@ int
 fhandler_registry::closedir (DIR * dir)
 {
   int res = 0;
-  if (dir->__handle != INVALID_HANDLE_VALUE &&
-      RegCloseKey ((HKEY) dir->__handle) != ERROR_SUCCESS)
+  if (dir->__handle != INVALID_HANDLE_VALUE)
     {
-      __seterrno ();
-      res = -1;
+      delete d_hash (dir);
+      if (RegCloseKey ((HKEY) dir->__handle) != ERROR_SUCCESS)
+ {
+  __seterrno ();
+  res = -1;
+ }
     }
   syscall_printf ("%d = closedir (%p)", res, dir);
   return 0;
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec  4 21:49, Christian Franke wrote:

> Here is a simple approach to handle the duplicate key/value name problem in
> /proc/registry. A value is skipped if key with same name exists. Number of
> actual key existence checks are reduced by a simple hash table.
>
> The patch also adds dirent.d_type support, find does no longer crash.
>
> Christian
>
>
> 2008-12-04  Christian Franke  <[hidden email]>
>
> * fhandler_registry.cc (__DIR_hash): New class.
> (d_hash): New macro.
> (key_exists): New function.
> (fhandler_registry::readdir): Allocate __DIR_hash.
> Record key names in hash table. Skip value if key
> with same name exists. Fix error handling of
> encode_regname (). Set dirent.d_type.
> (fhandler_registry::closedir): Delete __DIR_hash.

That looks like a quite neat idea to rectify this problem but, now that
I think of it I'm wondering if this isn't a good starting point for
a better solution as you proposed on the Cygwin list.

So let's assume there's a key and a value with the same name.

The old implementation just ignored the problem.  Trying to access the
value failed because the value was simply shadowed by the key.  `cat
foo' returned "is a directory" or something.

The now proposed solution hides the value instead.  There just isn't a
value of that name anymore.  In the end, the result is the same.
Accessing the value still doesn't work.

However, since these value were never accessible, doesn't that mean
there is no backward compatibility problem if we actually change the
name of the values instead to, say, foo.val?  That's what you proposed
on the main list, right?

Is the above line of thought correct?  If yes, together with your hash
table it would be quite simle to implement this.  We would just have to
think of a good value for ".val".  Unfortunately, there's no character
disallowed in the registry names, not even a \0 :(

Maybe ".val" is already a good suffix?


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Corinna Vinschen wrote:

> On Dec  4 21:49, Christian Franke wrote:
>  
>> Here is a simple approach to handle the duplicate key/value name problem in
>> /proc/registry. A value is skipped if key with same name exists. Number of
>> actual key existence checks are reduced by a simple hash table.
>>
>> ...
>>    
>
> That looks like a quite neat idea to rectify this problem but, now that
> I think of it I'm wondering if this isn't a good starting point for
> a better solution as you proposed on the Cygwin list.
>
>  
Yes, it is ... see below.


> So let's assume there's a key and a value with the same name.
>
> The old implementation just ignored the problem.  Trying to access the
> value failed because the value was simply shadowed by the key.  `cat
> foo' returned "is a directory" or something.
>
> The now proposed solution hides the value instead.  There just isn't a
> value of that name anymore.  In the end, the result is the same.
> Accessing the value still doesn't work.
>
>  
The hidden value also prevents that the key is scanned twice by find and ls.


> However, since these value were never accessible, doesn't that mean
> there is no backward compatibility problem if we actually change the
> name of the values instead to, say, foo.val?  That's what you proposed
> on the main list, right?
>
> Is the above line of thought correct?  If yes, together with your hash
> table it would be quite simle to implement this.  We would just have to
> think of a good value for ".val".  Unfortunately, there's no character
> disallowed in the registry names, not even a \0 :(
>
>  
Yes, and \0 is reportedly used at least by some copy protection software.


> Maybe ".val" is already a good suffix?
>
>  

I would prefer "%val" to avoid any extra encoding for names using
".val". The "%" is already used as an escape char, so "%val" in a name
would appear as "%25val"

With the attached patch, a duplicate name "foo" is handled as follows:

- readdir() returns the key as "foo" and the value as "foo%val".
- If the name is "foo%val", stat() and open() consider only the value "foo".

This keeps the names 'as is' if possible and allows access to the (very
few) entries with duplicate names. The "%val" is at least somewhat
self-explanatory.

Example:

$ ls -l
/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/ControlSet001/Services/Eventlog/Security
...
dr--r-x--- 3 Administratoren SYSTEM   0 Mar 29  2005 Security
dr--r-x--- 3 Administratoren SYSTEM   0 Mar 29  2005 Security Account
Manager
-r--r----- 1 Administratoren SYSTEM 168 Mar 29  2005 Security%val
...

Christian


diff --git a/winsup/cygwin/fhandler_registry.cc b/winsup/cygwin/fhandler_registry.cc
index ce4335f..4efe48e 100644
--- a/winsup/cygwin/fhandler_registry.cc
+++ b/winsup/cygwin/fhandler_registry.cc
@@ -91,7 +91,7 @@ must_encode (char c)
 /* Encode special chars in registry key or value name.
  */
 static int
-encode_regname (char * dst, const char * src)
+encode_regname (char * dst, const char * src, bool add_val)
 {
   int di = 0;
   for (int si = 0; src[si]; si++)
@@ -108,31 +108,47 @@ encode_regname (char * dst, const char * src)
       else
  dst[di++] = c;
     }
+
+  if (add_val)
+    {
+      if (di + 4 >= NAME_MAX + 1)
+ return ENAMETOOLONG;
+      memcpy (dst + di, "%val", 4);
+      di += 4;
+    }
+
   dst[di] = 0;
   return 0;
 }
 
 /* Decode special chars in registry key or value name.
+ * Returns 0: success, 1: "%val" detected, -1: error.
  */
 static int
 decode_regname (char * dst, const char * src, int len = -1)
 {
   if (len < 0)
     len = strlen (src);
+  int res = 0;
   int di = 0;
   for (int si = 0; si < len; si++)
     {
       char c = src[si];
       if (c == '%')
  {
+  if (si + 4 == len && !memcmp (src + si, "%val", 4))
+    {
+      res = 1;
+      break;
+    }
   if (si + 2 >= len)
-    return EINVAL;
+    return -1;
   char s[] = {src[si+1], src[si+2], '\0'};
   char *p;
   c = strtoul (s, &p, 16);
   if (!(must_encode (c) ||
         (c == '.' && si == 0 && (len == 3 || (src[3] == '.' && len == 4)))))
-    return EINVAL;
+    return -1;
   dst[di++] = c;
   si += 2;
  }
@@ -140,7 +156,49 @@ decode_regname (char * dst, const char * src, int len = -1)
  dst[di++] = c;
     }
   dst[di] = 0;
-  return 0;
+  return res;
+}
+
+
+/* Hash table to limit calls to key_exists ().
+ */
+class __DIR_hash
+{
+public:
+  __DIR_hash ()
+    {
+      memset (table, 0, sizeof(table));
+    }
+
+  void set (unsigned h)
+    {
+      table [(h >> 3) & (HASH_SIZE - 1)] |= (1 << (h & 0x3));
+    }
+
+  bool is_set (unsigned h) const
+    {
+      return (table [(h >> 3) & (HASH_SIZE - 1)] & (1 << (h & 0x3))) != 0;
+    }
+
+private:
+  enum { HASH_SIZE = 1024 };
+  unsigned char table[HASH_SIZE];
+};
+
+#define d_hash(d) ((__DIR_hash *) (d)->__d_internal)
+
+
+/* Return true if subkey NAME exists in key PARENT.
+ */
+static bool
+key_exists (HKEY parent, const char * name, DWORD wow64)
+{
+  HKEY hKey = (HKEY) INVALID_HANDLE_VALUE;
+  LONG error = RegOpenKeyEx (parent, name, 0, KEY_READ | wow64, &hKey);
+  if (error == ERROR_SUCCESS)
+    RegCloseKey (hKey);
+
+  return (error == ERROR_SUCCESS || error == ERROR_ACCESS_DENIED);
 }
 
 /* Returns 0 if path doesn't exist, >0 if path is a directory,
@@ -190,7 +248,13 @@ fhandler_registry::exists ()
       goto out;
     }
 
-  hKey = open_key (path, KEY_READ, wow64, false);
+  char dec_file[NAME_MAX + 1];
+  int val_only = decode_regname (dec_file, file);
+  if (val_only < 0)
+    goto out;
+
+  if (!val_only)
+    hKey = open_key (path, KEY_READ, wow64, false);
   if (hKey != (HKEY) INVALID_HANDLE_VALUE)
     file_type = 1;
   else
@@ -199,33 +263,36 @@ fhandler_registry::exists ()
       if (hKey == (HKEY) INVALID_HANDLE_VALUE)
  return 0;
 
-      while (ERROR_SUCCESS ==
-     (error = RegEnumKeyEx (hKey, index++, buf, &buf_size, NULL, NULL,
-     NULL, NULL))
-     || (error == ERROR_MORE_DATA))
+      if (!val_only)
  {
-  if (strcasematch (buf, file))
+  while (ERROR_SUCCESS ==
+ (error = RegEnumKeyEx (hKey, index++, buf, &buf_size,
+ NULL, NULL, NULL, NULL))
+ || (error == ERROR_MORE_DATA))
+    {
+      if (strcasematch (buf, dec_file))
+ {
+  file_type = 1;
+  goto out;
+ }
+ buf_size = NAME_MAX + 1;
+    }
+  if (error != ERROR_NO_MORE_ITEMS)
     {
-      file_type = 1;
+      seterrno_from_win_error (__FILE__, __LINE__, error);
       goto out;
     }
+  index = 0;
   buf_size = NAME_MAX + 1;
  }
-      if (error != ERROR_NO_MORE_ITEMS)
- {
-  seterrno_from_win_error (__FILE__, __LINE__, error);
-  goto out;
- }
-      index = 0;
-      buf_size = NAME_MAX + 1;
+
       while (ERROR_SUCCESS ==
      (error = RegEnumValue (hKey, index++, buf, &buf_size, NULL, NULL,
     NULL, NULL))
      || (error == ERROR_MORE_DATA))
  {
-  char enc_buf[NAME_MAX + 1];
   if (   (buf[0] == '\0' && strcasematch (file, DEFAULT_VALUE_NAME))
-      || (!encode_regname (enc_buf, buf) && strcasematch (enc_buf, file)))
+      || strcasematch (buf, dec_file))
     {
       file_type = -1;
       goto out;
@@ -324,7 +391,7 @@ fhandler_registry::fstat (struct __stat64 *buf)
   value_name++;
   char dec_value_name[NAME_MAX + 1];
   DWORD dwSize;
-  if (!decode_regname (dec_value_name, value_name) &&
+  if (decode_regname (dec_value_name, value_name) >= 0 &&
       ERROR_SUCCESS ==
       RegQueryValueEx (hKey, dec_value_name, NULL, NULL, NULL,
        &dwSize))
@@ -381,13 +448,16 @@ fhandler_registry::readdir (DIR *dir, dirent *de)
       res = 0;
       goto out;
     }
-  if (dir->__handle == INVALID_HANDLE_VALUE && dir->__d_position == 0)
+  if (dir->__handle == INVALID_HANDLE_VALUE)
     {
+      if (dir->__d_position != 0)
+ goto out;
       handle = open_key (path + 1, KEY_READ, wow64, false);
       dir->__handle = handle;
+      if (dir->__handle == INVALID_HANDLE_VALUE)
+ goto out;
+      dir->__d_internal = (unsigned) new __DIR_hash ();
     }
-  if (dir->__handle == INVALID_HANDLE_VALUE)
-    goto out;
   if (dir->__d_position < SPECIAL_DOT_FILE_COUNT)
     {
       strcpy (de->d_name, special_dot_files[dir->__d_position++]);
@@ -425,14 +495,35 @@ retry:
     }
 
   /* We get here if `buf' contains valid data.  */
+  dir->__d_position++;
+  if (dir->__d_position & REG_ENUM_VALUES_MASK)
+    dir->__d_position += 0x10000;
+
   if (*buf == 0)
     strcpy (de->d_name, DEFAULT_VALUE_NAME);
-  else if (encode_regname (de->d_name, buf))
-    goto retry;
+  else
+    {
+      /* Append "%val" if value name is identical to a previous key name.  */
+      unsigned h = hash_path_name (1, buf);
+      bool add_val = false;
+      if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
+ d_hash (dir)->set (h);
+      else if (d_hash (dir)->is_set (h)
+       && key_exists ((HKEY) dir->__handle, buf, wow64))
+ add_val = true;
+
+      if (encode_regname (de->d_name, buf, add_val))
+ {
+  buf_size = NAME_MAX + 1;
+  goto retry;
+ }
+    }
 
-  dir->__d_position++;
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
-    dir->__d_position += 0x10000;
+    de->d_type = DT_REG;
+  else
+    de->d_type = DT_DIR;
+
   res = 0;
 out:
   syscall_printf ("%d = readdir (%p, %p)", res, dir, de);
@@ -473,11 +564,14 @@ int
 fhandler_registry::closedir (DIR * dir)
 {
   int res = 0;
-  if (dir->__handle != INVALID_HANDLE_VALUE &&
-      RegCloseKey ((HKEY) dir->__handle) != ERROR_SUCCESS)
+  if (dir->__handle != INVALID_HANDLE_VALUE)
     {
-      __seterrno ();
-      res = -1;
+      delete d_hash (dir);
+      if (RegCloseKey ((HKEY) dir->__handle) != ERROR_SUCCESS)
+ {
+  __seterrno ();
+  res = -1;
+ }
     }
   syscall_printf ("%d = closedir (%p)", res, dir);
   return 0;
@@ -488,7 +582,7 @@ fhandler_registry::open (int flags, mode_t mode)
 {
   int pathlen;
   const char *file;
-  HKEY handle;
+  HKEY handle = (HKEY) INVALID_HANDLE_VALUE;
 
   int res = fhandler_virtual::open (flags, mode);
   if (!res)
@@ -573,14 +667,16 @@ fhandler_registry::open (int flags, mode_t mode)
     }
 
   char dec_file[NAME_MAX + 1];
-  if (decode_regname (dec_file, file))
+  int val_only = decode_regname (dec_file, file);
+  if (val_only < 0)
     {
       set_errno (EINVAL);
       res = 0;
       goto out;
     }
 
-  handle = open_key (path, KEY_READ, wow64, false);
+  if (!val_only)
+    handle = open_key (path, KEY_READ, wow64, false);
   if (handle == (HKEY) INVALID_HANDLE_VALUE)
     {
       handle = open_key (path, KEY_READ, wow64, true);
@@ -736,7 +832,8 @@ open_key (const char *name, REGSAM access, DWORD wow64, bool isValue)
       const char *anchor = name;
       while (*name && !isdirsep (*name))
  name++;
-      if (decode_regname (component, anchor, name - anchor))
+      int val_only = decode_regname (component, anchor, name - anchor);
+      if (val_only < 0)
         {
   set_errno (EINVAL);
   if (parentOpened)
@@ -749,6 +846,15 @@ open_key (const char *name, REGSAM access, DWORD wow64, bool isValue)
       if (*name == 0 && isValue == true)
  goto out;
 
+      if (val_only)
+ {
+  set_errno (ENOENT);
+  if (parentOpened)
+    RegCloseKey (hParentKey);
+  hKey = (HKEY) INVALID_HANDLE_VALUE;
+  break;
+ }
+
       if (hParentKey != (HKEY) INVALID_HANDLE_VALUE)
  {
   REGSAM effective_access = KEY_READ;
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
Hi Christian,

On Dec  5 23:23, Christian Franke wrote:
> Corinna Vinschen wrote:
>> Maybe ".val" is already a good suffix?
>
> I would prefer "%val" to avoid any extra encoding for names using ".val".
> The "%" is already used as an escape char, so "%val" in a name would appear
> as "%25val"

Very good idea!

> With the attached patch, a duplicate name "foo" is handled as follows:
>
> - readdir() returns the key as "foo" and the value as "foo%val".
> - If the name is "foo%val", stat() and open() consider only the value
> "foo".
>
> This keeps the names 'as is' if possible and allows access to the (very
> few) entries with duplicate names. The "%val" is at least somewhat
> self-explanatory.

Cool.  Can you please send a ChangeLog entry as well?


Thanks,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Hi Corinna,

Corinna Vinschen wrote:

> ...
>> With the attached patch, a duplicate name "foo" is handled as follows:
>>
>> - readdir() returns the key as "foo" and the value as "foo%val".
>> - If the name is "foo%val", stat() and open() consider only the value
>> "foo".
>>
>> This keeps the names 'as is' if possible and allows access to the (very
>> few) entries with duplicate names. The "%val" is at least somewhat
>> self-explanatory.
>>    
>
> Cool.  Can you please send a ChangeLog entry as well?
>
>  

Of course:

2008-12-07  Christian Franke  <[hidden email]>

        * fhandler_registry.cc (encode_regname): Add Parameter add_val.
        Append "%val" if add_val is set.
        (decode_regname): Remove trailing "%val". Change returncode
        accordingly.
        (__DIR_hash): New class.
        (d_hash): New macro.
        (key_exists): New function.
        (fhandler_registry::exists): Remove encode of registry name
        before path compare, decode file part of path instead.
        Skip checks for keys if trailing "%val" detected.
        (fhandler_registry::fstat): Change check of return
        value of decode_regname ().
        (fhandler_registry::readdir): Allocate __DIR_hash.
        Record key names in hash table. Append "%val" if key with
        same name exists. Fix error handling of encode_regname ().
        Set dirent.d_type.
        (fhandler_registry::closedir): Delete __DIR_hash.
        (fhandler_registry::open): Don't open key if trailing "%val"
        detected by decode_regname ().
        (open_key): Ditto.



Christian

Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec  7 20:03, Christian Franke wrote:

> Corinna Vinschen wrote:
>> ...
>>> With the attached patch, a duplicate name "foo" is handled as follows:
>>>
>>> - readdir() returns the key as "foo" and the value as "foo%val".
>>> - If the name is "foo%val", stat() and open() consider only the value
>>> "foo".
>>[...]
>> Cool.  Can you please send a ChangeLog entry as well?
>
> Of course:

Thanks!  Patch applied.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec  8 12:48, Corinna Vinschen wrote:

> On Dec  7 20:03, Christian Franke wrote:
> > Corinna Vinschen wrote:
> >> ...
> >>> With the attached patch, a duplicate name "foo" is handled as follows:
> >>>
> >>> - readdir() returns the key as "foo" and the value as "foo%val".
> >>> - If the name is "foo%val", stat() and open() consider only the value
> >>> "foo".
> >>[...]
> >> Cool.  Can you please send a ChangeLog entry as well?
> >
> > Of course:
>
> Thanks!  Patch applied.

Oh, btw.

I was wondering if you would be not too disgusted by the idea to add
some documentation about this change to the Cygwin User's Guide.
There's already some blurb in pathnames.sgml about the /proc/registry
access.  Currently it lacks a description of the entire % handling.
Maybe it would be helpful to break out an entire (small) section for the
/proc/registry access...


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Corinna Vinschen wrote:

>
> Oh, btw.
>
> I was wondering if you would be not too disgusted by the idea to add
> some documentation about this change to the Cygwin User's Guide.
> There's already some blurb in pathnames.sgml about the /proc/registry
> access.  Currently it lacks a description of the entire % handling.
> Maybe it would be helpful to break out an entire (small) section for the
> /proc/registry access...
>
>  

2008-12-11  Christian Franke  <[hidden email]>

        * pathnames.sgml: New section for /proc/registry. Document registry
        name encoding.


Christian



diff --git a/winsup/doc/pathnames.sgml b/winsup/doc/pathnames.sgml
index 2daad6d..f501606 100644
--- a/winsup/doc/pathnames.sgml
+++ b/winsup/doc/pathnames.sgml
@@ -510,11 +510,23 @@ displays information such as what model and speed processor you have.
 </para>
 <para>
 One unique aspect of the Cygwin <filename>/proc</filename> filesystem
-is <filename>/proc/registry</filename>, which displays the Windows
-registry with each <literal>KEY</literal> as a directory and each
-<literal>VALUE</literal> as a file. As anytime you deal with the
-Windows registry, use caution since changes may result in an unstable
-or broken system.  There are additionally subdirectories called
+is <filename>/proc/registry</filename>, see next section.
+</para>
+<para>
+The Cygwin <filename>/proc</filename> is not as complete as the
+one in Linux, but it provides significant capabilities. The
+<systemitem>procps</systemitem> package contains several utilities
+that use it.
+</para>
+</sect2>
+
+<sect2 id="pathnames-proc-registry"><title>The /proc/registry filesystem</title>
+<para>
+The <filename>/proc/registry</filename> filesystem provides read-only
+access to the Windows registry.  It displays each <literal>KEY</literal>
+as a directory and each <literal>VALUE</literal> as a file.  As anytime
+you deal with the Windows registry, use caution since changes may result
+in an unstable or broken system.  There are additionally subdirectories called
 <filename>/proc/registry32</filename> and <filename>/proc/registry64</filename>.
 They are identical to <filename>/proc/registry</filename> on 32 bit
 host OSes.  On 64 bit host OSes, <filename>/proc/registry32</filename>
@@ -522,10 +534,29 @@ opens the 32 bit processes view on the registry, while
 <filename>/proc/registry64</filename> opens the 64 bit processes view.
 </para>
 <para>
-The Cygwin <filename>/proc</filename> is not as complete as the
-one in Linux, but it provides significant capabilities. The
-<systemitem>procps</systemitem> package contains several utilities
-that use it.
+Reserved characters ('/', '\', ':', and '%') or reserved names
+(<filename>.</filename> and <filename>..</filename>) are converted by
+percent-encoding:
+<screen>
+<prompt>bash$</prompt> <userinput>regtool list -v '\HKEY_LOCAL_MACHINE\SYSTEM\MountedDevices'</userinput>
+...
+\DosDevices\C: (REG_BINARY) = cf a8 97 e8 00 08 fe f7
+...
+<prompt>bash$</prompt> <userinput>cd /proc/registry/HKEY_LOCAL_MACHINE/SYSTEM</userinput>
+<prompt>bash$</prompt> <userinput>ls -l MountedDevices</userinput>
+...
+-r--r----- 1 Admin SYSTEM  12 Dec 10 11:20 %5CDosDevices%5CC%3A
+...
+<prompt>bash$</prompt> <userinput>od -t x1 MountedDevices/%5CDosDevices%5CC%3A</userinput>
+0000000 cf a8 97 e8 00 08 fe f7 01 00 00 00
+</screen>
+The unnamed (default) value of a key can be accessed using the filename
+<filename>@</filename>.
+</para>
+<para>
+If a registry key contains a subkey and a value with the same name
+<filename>foo</filename>, Cygwin displays the subkey as
+<filename>foo</filename> and the value as <filename>foo%val</filename>.
 </para>
 </sect2>
 
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 11 21:20, Christian Franke wrote:

> Corinna Vinschen wrote:
>>
>> Oh, btw.
>>
>> I was wondering if you would be not too disgusted by the idea to add
>> some documentation about this change to the Cygwin User's Guide.
>> There's already some blurb in pathnames.sgml about the /proc/registry
>> access.  Currently it lacks a description of the entire % handling.
>> Maybe it would be helpful to break out an entire (small) section for the
>> /proc/registry access...
>>
>>  
>
>
> 2008-12-11  Christian Franke  <[hidden email]>
>
> * pathnames.sgml: New section for /proc/registry. Document registry
> name encoding.

Cool, thank you!  Patch applied.

Here's a question which occured to me when reading the doc after I had
applied it.  There's apparently still a problem which is, how do you
read the default value of a key if a value called '@' exists?  Do you
have an idea for a simple solution?


Thanks again,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Eric Blake (cygwin)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Corinna Vinschen on 12/12/2008 8:20 AM:
>
> Here's a question which occured to me when reading the doc after I had
> applied it.  There's apparently still a problem which is, how do you
> read the default value of a key if a value called '@' exists?  Do you
> have an idea for a simple solution?

"@" for the named value, and "%.val" for the unnamed default?

- --
Don't work too hard, make some time for fun as well!

Eric Blake             [hidden email]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklCh/QACgkQ84KuGfSFAYDg9gCfdlY71wkx9/99qqcJazGrdRTY
n5cAoIvh7tJGy99lOjNecBlT6TkcwXxc
=dJiq
-----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 12 08:49, Eric Blake wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> According to Corinna Vinschen on 12/12/2008 8:20 AM:
> >
> > Here's a question which occured to me when reading the doc after I had
> > applied it.  There's apparently still a problem which is, how do you
> > read the default value of a key if a value called '@' exists?  Do you
> > have an idea for a simple solution?
>
> "@" for the named value, and "%.val" for the unnamed default?

Backward compatibility would ask for sticking to @ for the default
value.  Actually there could be a key and a value called @ so you
have three @ items. :-P


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Eric Blake (cygwin)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Corinna Vinschen on 12/12/2008 9:13 AM:
>> "@" for the named value, and "%.val" for the unnamed default?
>
> Backward compatibility would ask for sticking to @ for the default
> value.  Actually there could be a key and a value called @ so you
> have three @ items. :-P

If there is no key or value @, then use @ for the default for
compatibility.  If there is either a key or a value named @, then use:

@ - named key
@%val - named value
%val - default value

- --
Don't work too hard, make some time for fun as well!

Eric Blake             [hidden email]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklCjqQACgkQ84KuGfSFAYA9MACgkdlmxZOiNCMfe700l0KdUf+X
DnEAnRGxplpN33GTEzKHqrx4uufIeIhG
=fpmG
-----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 12 09:17, Eric Blake wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> According to Corinna Vinschen on 12/12/2008 9:13 AM:
> >> "@" for the named value, and "%.val" for the unnamed default?
> >
> > Backward compatibility would ask for sticking to @ for the default
> > value.  Actually there could be a key and a value called @ so you
> > have three @ items. :-P
>
> If there is no key or value @, then use @ for the default for
> compatibility.  If there is either a key or a value named @, then use:
>
> @ - named key
> @%val - named value
> %val - default value

Something like that, I guess, though it I get headaches imagining that
the default value is not the default value anymore if by chance a @ key
or value exists.  It's a pity that we didn't have Christian's patch
right from the start.  I'm just glad that this is a seldom border case.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Corinna Vinschen wrote:

> On Dec 12 09:17, Eric Blake wrote:
>  
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> According to Corinna Vinschen on 12/12/2008 9:13 AM:
>>    
>>>> "@" for the named value, and "%.val" for the unnamed default?
>>>>        
>>> Backward compatibility would ask for sticking to @ for the default
>>> value.  Actually there could be a key and a value called @ so you
>>> have three @ items. :-P
>>>      
>> If there is no key or value @, then use @ for the default for
>> compatibility.  If there is either a key or a value named @, then use:
>>
>> @ - named key
>> @%val - named value
>> %val - default value
>>    
>
> Something like that, I guess, though it I get headaches imagining that
> the default value is not the default value anymore if by chance a @ key
> or value exists.  It's a pity that we didn't have Christian's patch
> right from the start.  I'm just glad that this is a seldom border case.
>
>
>  

Why not encode "@" as a reserved name like it is already done for "."
and ".." (which appear as "%2E" and "%2E.")? This would provide backward
compatibility and consistency with current conversions:

@ - default value
%40 - named key or value
%40%val - named value if key exists

I will post a patch.

Christian

Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 12 18:32, Christian Franke wrote:
> Why not encode "@" as a reserved name like it is already done for "." and
> ".." (which appear as "%2E" and "%2E.")? This would provide backward
> compatibility and consistency with current conversions:
>
> @ - default value
> %40 - named key or value
> %40%val - named value if key exists
>
> I will post a patch.

Perfect.


Thamks,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Corinna Vinschen wrote:

> On Dec 12 18:32, Christian Franke wrote:
>  
>> Why not encode "@" as a reserved name like it is already done for "." and
>> ".." (which appear as "%2E" and "%2E.")? This would provide backward
>> compatibility and consistency with current conversions:
>>
>> @ - default value
>> %40 - named key or value
>> %40%val - named value if key exists
>>
>> I will post a patch.
>>    
>
> Perfect.
>
>  
As a side effect, the patch fixes an old bug:
stat("/proc/registry/.../@", .) did not set st_size.


2008-12-13  Christian Franke  <[hidden email]>

        * fhandler_registry.cc (DEFAULT_VALUE_NAME): Remove constant.
        (encode_regname): Encode empty (default) name to "@".
        Encode "@" to "%40".  Change error return to -1.
        (decode_regname): Decode "@" to empty name.  Decode "%40" to "@".
        (fhandler_registry::exists): Skip check for keys if name is empty.
        Remove check for DEFAULT_VALUE_NAME, now handled by decode_regname ().
        (fhandler_registry::readdir): Remove check for empty name, now
        handled by encode_regname ().
        (fhandler_registry::open): Remove check for DEFAULT_VALUE_NAME.
        (fhandler_registry::open_key): Fail with ENOENT if key name is empty.

Christian



diff --git a/winsup/cygwin/fhandler_registry.cc b/winsup/cygwin/fhandler_registry.cc
index 4efe48e..17d9109 100644
--- a/winsup/cygwin/fhandler_registry.cc
+++ b/winsup/cygwin/fhandler_registry.cc
@@ -75,9 +75,6 @@ static const char *special_dot_files[] =
 static const int SPECIAL_DOT_FILE_COUNT =
   (sizeof (special_dot_files) / sizeof (const char *)) - 1;
 
-/* Name given to default values */
-static const char *DEFAULT_VALUE_NAME = "@";
-
 static HKEY open_key (const char *name, REGSAM access, DWORD wow64, bool isValue);
 
 /* Return true if char must be encoded.
@@ -89,30 +86,35 @@ must_encode (char c)
 }
 
 /* Encode special chars in registry key or value name.
+ * Returns 0: success, -1: error.
  */
 static int
 encode_regname (char * dst, const char * src, bool add_val)
 {
   int di = 0;
-  for (int si = 0; src[si]; si++)
-    {
-      char c = src[si];
-      if (must_encode (c) ||
-  (c == '.' && si == 0 && (!src[1] || (src[1] == '.' && !src[2]))))
- {
-  if (di + 3 >= NAME_MAX + 1)
-    return ENAMETOOLONG;
-  __small_sprintf (dst + di, "%%%02x", c);
-  di += 3;
- }
-      else
- dst[di++] = c;
-    }
+  if (!src[0])
+    dst[di++] = '@'; // Default value.
+  else
+    for (int si = 0; src[si]; si++)
+      {
+ char c = src[si];
+ if (must_encode (c) ||
+    (si == 0 && ((c == '.' && (!src[1] || (src[1] == '.' && !src[2]))) ||
+ (c == '@' && !src[1]))))
+  {
+    if (di + 3 >= NAME_MAX + 1)
+      return -1;
+    __small_sprintf (dst + di, "%%%02x", c);
+    di += 3;
+  }
+ else
+  dst[di++] = c;
+      }
 
   if (add_val)
     {
       if (di + 4 >= NAME_MAX + 1)
- return ENAMETOOLONG;
+ return -1;
       memcpy (dst + di, "%val", 4);
       di += 4;
     }
@@ -129,32 +131,39 @@ decode_regname (char * dst, const char * src, int len = -1)
 {
   if (len < 0)
     len = strlen (src);
+
   int res = 0;
-  int di = 0;
-  for (int si = 0; si < len; si++)
+  if (len > 4 && !memcmp (src + len - 4, "%val", 4))
     {
-      char c = src[si];
-      if (c == '%')
- {
-  if (si + 4 == len && !memcmp (src + si, "%val", 4))
-    {
-      res = 1;
-      break;
-    }
-  if (si + 2 >= len)
-    return -1;
-  char s[] = {src[si+1], src[si+2], '\0'};
-  char *p;
-  c = strtoul (s, &p, 16);
-  if (!(must_encode (c) ||
-        (c == '.' && si == 0 && (len == 3 || (src[3] == '.' && len == 4)))))
-    return -1;
-  dst[di++] = c;
-  si += 2;
- }
-      else
- dst[di++] = c;
+      len -= 4;
+      res = 1;
     }
+
+  int di = 0;
+  if (len == 1 && src[0] == '@')
+    ; // Default value.
+  else
+    for (int si = 0; si < len; si++)
+      {
+ char c = src[si];
+ if (c == '%')
+  {
+    if (si + 2 >= len)
+      return -1;
+    char s[] = {src[si+1], src[si+2], '\0'};
+    char *p;
+    c = strtoul (s, &p, 16);
+    if (!(must_encode (c) ||
+  (si == 0 && ((c == '.' && (len == 3 || (src[3] == '.' && len == 4))) ||
+       (c == '@' && len == 3)))))
+      return -1;
+    dst[di++] = c;
+    si += 2;
+  }
+ else
+  dst[di++] = c;
+      }
+
   dst[di] = 0;
   return res;
 }
@@ -263,7 +272,7 @@ fhandler_registry::exists ()
       if (hKey == (HKEY) INVALID_HANDLE_VALUE)
  return 0;
 
-      if (!val_only)
+      if (!val_only && dec_file[0])
  {
   while (ERROR_SUCCESS ==
  (error = RegEnumKeyEx (hKey, index++, buf, &buf_size,
@@ -291,8 +300,7 @@ fhandler_registry::exists ()
     NULL, NULL))
      || (error == ERROR_MORE_DATA))
  {
-  if (   (buf[0] == '\0' && strcasematch (file, DEFAULT_VALUE_NAME))
-      || strcasematch (buf, dec_file))
+  if (strcasematch (buf, dec_file))
     {
       file_type = -1;
       goto out;
@@ -499,25 +507,22 @@ retry:
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
     dir->__d_position += 0x10000;
 
-  if (*buf == 0)
-    strcpy (de->d_name, DEFAULT_VALUE_NAME);
-  else
-    {
-      /* Append "%val" if value name is identical to a previous key name.  */
-      unsigned h = hash_path_name (1, buf);
-      bool add_val = false;
-      if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
- d_hash (dir)->set (h);
-      else if (d_hash (dir)->is_set (h)
-       && key_exists ((HKEY) dir->__handle, buf, wow64))
- add_val = true;
-
-      if (encode_regname (de->d_name, buf, add_val))
- {
-  buf_size = NAME_MAX + 1;
-  goto retry;
- }
-    }
+  {
+    /* Append "%val" if value name is identical to a previous key name.  */
+    unsigned h = hash_path_name (1, buf);
+    bool add_val = false;
+    if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
+      d_hash (dir)->set (h);
+    else if (d_hash (dir)->is_set (h)
+     && key_exists ((HKEY) dir->__handle, buf, wow64))
+      add_val = true;
+
+    if (encode_regname (de->d_name, buf, add_val))
+      {
+ buf_size = NAME_MAX + 1;
+ goto retry;
+      }
+  }
 
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
     de->d_type = DT_REG;
@@ -690,11 +695,7 @@ fhandler_registry::open (int flags, mode_t mode)
     flags |= O_DIROPEN;
 
   set_io_handle (handle);
-
-  if (strcasematch (dec_file, DEFAULT_VALUE_NAME))
-    value_name = cstrdup ("");
-  else
-    value_name = cstrdup (dec_file);
+  value_name = cstrdup (dec_file);
 
   if (!(flags & O_DIROPEN) && !fill_filebuf ())
     {
@@ -846,7 +847,7 @@ open_key (const char *name, REGSAM access, DWORD wow64, bool isValue)
       if (*name == 0 && isValue == true)
  goto out;
 
-      if (val_only)
+      if (val_only || !component[0])
  {
   set_errno (ENOENT);
   if (parentOpened)
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 13 14:37, Christian Franke wrote:

> Corinna Vinschen wrote:
>> On Dec 12 18:32, Christian Franke wrote:
>>  
>>> Why not encode "@" as a reserved name like it is already done for "." and
>>> ".." (which appear as "%2E" and "%2E.")? This would provide backward
>>> compatibility and consistency with current conversions:
>>>
>>> @ - default value
>>> %40 - named key or value
>>> %40%val - named value if key exists
>>>
>>> I will post a patch.
>>>    
>>
>> Perfect.
>>
>>  
>
> As a side effect, the patch fixes an old bug:
> stat("/proc/registry/.../@", .) did not set st_size.
>
>
> 2008-12-13  Christian Franke  <[hidden email]>
>
> * fhandler_registry.cc (DEFAULT_VALUE_NAME): Remove constant.
> (encode_regname): Encode empty (default) name to "@".
> Encode "@" to "%40".  Change error return to -1.
> (decode_regname): Decode "@" to empty name.  Decode "%40" to "@".
> (fhandler_registry::exists): Skip check for keys if name is empty.
> Remove check for DEFAULT_VALUE_NAME, now handled by decode_regname ().
> (fhandler_registry::readdir): Remove check for empty name, now
> handled by encode_regname ().
> (fhandler_registry::open): Remove check for DEFAULT_VALUE_NAME.
> (fhandler_registry::open_key): Fail with ENOENT if key name is empty.

Thanks for the patch.  Can you resend it against the latest version
of fhandler_registry.cc, please?  It doesn't apply cleanly anymore.


Thanks,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Christian Franke
Corinna Vinschen wrote:

> On Dec 13 14:37, Christian Franke wrote:
>  
>>
>> 2008-12-13  Christian Franke  <[hidden email]>
>>
>> * fhandler_registry.cc (DEFAULT_VALUE_NAME): Remove constant.
>> (encode_regname): Encode empty (default) name to "@".
>> Encode "@" to "%40".  Change error return to -1.
>> (decode_regname): Decode "@" to empty name.  Decode "%40" to "@".
>> (fhandler_registry::exists): Skip check for keys if name is empty.
>> Remove check for DEFAULT_VALUE_NAME, now handled by decode_regname ().
>> (fhandler_registry::readdir): Remove check for empty name, now
>> handled by encode_regname ().
>> (fhandler_registry::open): Remove check for DEFAULT_VALUE_NAME.
>> (fhandler_registry::open_key): Fail with ENOENT if key name is empty.
>>    
>
> Thanks for the patch.  Can you resend it against the latest version
> of fhandler_registry.cc, please?  It doesn't apply cleanly anymore.
>
>
>  
New patch is attached, changelog is still valid.

Christian


diff --git a/winsup/cygwin/fhandler_registry.cc b/winsup/cygwin/fhandler_registry.cc
index cfe4c07..9efb2d1 100644
--- a/winsup/cygwin/fhandler_registry.cc
+++ b/winsup/cygwin/fhandler_registry.cc
@@ -75,9 +75,6 @@ static const char *special_dot_files[] =
 static const int SPECIAL_DOT_FILE_COUNT =
   (sizeof (special_dot_files) / sizeof (const char *)) - 1;
 
-/* Name given to default values */
-static const char *DEFAULT_VALUE_NAME = "@";
-
 static HKEY open_key (const char *name, REGSAM access, DWORD wow64, bool isValue);
 
 /* Return true if char must be encoded.
@@ -89,30 +86,35 @@ must_encode (char c)
 }
 
 /* Encode special chars in registry key or value name.
+ * Returns 0: success, -1: error.
  */
 static int
 encode_regname (char * dst, const char * src, bool add_val)
 {
   int di = 0;
-  for (int si = 0; src[si]; si++)
-    {
-      char c = src[si];
-      if (must_encode (c) ||
-  (c == '.' && si == 0 && (!src[1] || (src[1] == '.' && !src[2]))))
- {
-  if (di + 3 >= NAME_MAX + 1)
-    return ENAMETOOLONG;
-  __small_sprintf (dst + di, "%%%02x", c);
-  di += 3;
- }
-      else
- dst[di++] = c;
-    }
+  if (!src[0])
+    dst[di++] = '@'; // Default value.
+  else
+    for (int si = 0; src[si]; si++)
+      {
+ char c = src[si];
+ if (must_encode (c) ||
+    (si == 0 && ((c == '.' && (!src[1] || (src[1] == '.' && !src[2]))) ||
+ (c == '@' && !src[1]))))
+  {
+    if (di + 3 >= NAME_MAX + 1)
+      return -1;
+    __small_sprintf (dst + di, "%%%02x", c);
+    di += 3;
+  }
+ else
+  dst[di++] = c;
+      }
 
   if (add_val)
     {
       if (di + 4 >= NAME_MAX + 1)
- return ENAMETOOLONG;
+ return -1;
       memcpy (dst + di, "%val", 4);
       di += 4;
     }
@@ -129,32 +131,39 @@ decode_regname (char * dst, const char * src, int len = -1)
 {
   if (len < 0)
     len = strlen (src);
+
   int res = 0;
-  int di = 0;
-  for (int si = 0; si < len; si++)
+  if (len > 4 && !memcmp (src + len - 4, "%val", 4))
     {
-      char c = src[si];
-      if (c == '%')
- {
-  if (si + 4 == len && !memcmp (src + si, "%val", 4))
-    {
-      res = 1;
-      break;
-    }
-  if (si + 2 >= len)
-    return -1;
-  char s[] = {src[si+1], src[si+2], '\0'};
-  char *p;
-  c = strtoul (s, &p, 16);
-  if (!(must_encode (c) ||
- (c == '.' && si == 0 && (len == 3 || (src[3] == '.' && len == 4)))))
-    return -1;
-  dst[di++] = c;
-  si += 2;
- }
-      else
- dst[di++] = c;
+      len -= 4;
+      res = 1;
     }
+
+  int di = 0;
+  if (len == 1 && src[0] == '@')
+    ; // Default value.
+  else
+    for (int si = 0; si < len; si++)
+      {
+ char c = src[si];
+ if (c == '%')
+  {
+    if (si + 2 >= len)
+      return -1;
+    char s[] = {src[si+1], src[si+2], '\0'};
+    char *p;
+    c = strtoul (s, &p, 16);
+    if (!(must_encode (c) ||
+  (si == 0 && ((c == '.' && (len == 3 || (src[3] == '.' && len == 4))) ||
+       (c == '@' && len == 3)))))
+      return -1;
+    dst[di++] = c;
+    si += 2;
+  }
+ else
+  dst[di++] = c;
+      }
+
   dst[di] = 0;
   return res;
 }
@@ -264,7 +273,7 @@ fhandler_registry::exists ()
   if (hKey == (HKEY) INVALID_HANDLE_VALUE)
     return 0;
 
-  if (!val_only)
+  if (!val_only && dec_file[0])
     {
       while (ERROR_SUCCESS ==
      (error = RegEnumKeyEx (hKey, index++, buf, &buf_size,
@@ -292,8 +301,7 @@ fhandler_registry::exists ()
  NULL, NULL))
  || (error == ERROR_MORE_DATA))
     {
-      if (   (buf[0] == '\0' && strcasematch (file, DEFAULT_VALUE_NAME))
-  || strcasematch (buf, dec_file))
+      if (strcasematch (buf, dec_file))
  {
   file_type = -1;
   goto out;
@@ -501,25 +509,22 @@ retry:
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
     dir->__d_position += 0x10000;
 
-  if (*buf == 0)
-    strcpy (de->d_name, DEFAULT_VALUE_NAME);
-  else
-    {
-      /* Append "%val" if value name is identical to a previous key name.  */
-      unsigned h = hash_path_name (1, buf);
-      bool add_val = false;
-      if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
- d_hash (dir)->set (h);
-      else if (d_hash (dir)->is_set (h)
-       && key_exists ((HKEY) dir->__handle, buf, wow64))
- add_val = true;
-
-      if (encode_regname (de->d_name, buf, add_val))
- {
-  buf_size = NAME_MAX + 1;
-  goto retry;
- }
-    }
+  {
+    /* Append "%val" if value name is identical to a previous key name.  */
+    unsigned h = hash_path_name (1, buf);
+    bool add_val = false;
+    if (! (dir->__d_position & REG_ENUM_VALUES_MASK))
+      d_hash (dir)->set (h);
+    else if (d_hash (dir)->is_set (h)
+     && key_exists ((HKEY) dir->__handle, buf, wow64))
+      add_val = true;
+
+    if (encode_regname (de->d_name, buf, add_val))
+      {
+ buf_size = NAME_MAX + 1;
+ goto retry;
+      }
+  }
 
   if (dir->__d_position & REG_ENUM_VALUES_MASK)
     de->d_type = DT_REG;
@@ -695,11 +700,7 @@ fhandler_registry::open (int flags, mode_t mode)
  flags |= O_DIROPEN;
 
       set_io_handle (handle);
-
-      if (strcasematch (dec_file, DEFAULT_VALUE_NAME))
- value_name = cstrdup ("");
-      else
- value_name = cstrdup (dec_file);
+      value_name = cstrdup (dec_file);
 
       if (!(flags & O_DIROPEN) && !fill_filebuf ())
  {
@@ -852,7 +853,7 @@ open_key (const char *name, REGSAM access, DWORD wow64, bool isValue)
       if (*name == 0 && isValue == true)
  goto out;
 
-      if (val_only)
+      if (val_only || !component[0])
  {
   set_errno (ENOENT);
   if (parentOpened)
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] Avoid duplicate names in /proc/registry (which may crash find)

Corinna Vinschen-2
On Dec 15 21:31, Christian Franke wrote:

> Corinna Vinschen wrote:
>> On Dec 13 14:37, Christian Franke wrote:
>>  
>>>
>>> 2008-12-13  Christian Franke  <[hidden email]>
>>>
>>> * fhandler_registry.cc (DEFAULT_VALUE_NAME): Remove constant.
>>> (encode_regname): Encode empty (default) name to "@".
>>> Encode "@" to "%40".  Change error return to -1.
>>> (decode_regname): Decode "@" to empty name.  Decode "%40" to "@".
>>> (fhandler_registry::exists): Skip check for keys if name is empty.
>>> Remove check for DEFAULT_VALUE_NAME, now handled by decode_regname ().
>>> (fhandler_registry::readdir): Remove check for empty name, now
>>> handled by encode_regname ().
>>> (fhandler_registry::open): Remove check for DEFAULT_VALUE_NAME.
>>> (fhandler_registry::open_key): Fail with ENOENT if key name is empty.
>>>    
>>
>> Thanks for the patch.  Can you resend it against the latest version
>> of fhandler_registry.cc, please?  It doesn't apply cleanly anymore.
>>
>>
>>  
>
> New patch is attached, changelog is still valid.

Patch applied.  Thanks!


Corinna


--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat