unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] getmntent: Generalize octal decoding
@ 2020-12-16 14:24 Siddhesh Poyarekar via Libc-alpha
  2020-12-22  7:48 ` [PING][PATCH] " Siddhesh Poyarekar
  0 siblings, 1 reply; 2+ messages in thread
From: Siddhesh Poyarekar via Libc-alpha @ 2020-12-16 14:24 UTC (permalink / raw
  To: libc-alpha; +Cc: fweimer

The current octal decoding looks for specific characters to decode.
generalize this instead, allowing any arbitrary \xxx octal pattern to
be decoded as long as it is a printable character.

The decoding is now done a bit later so that it does not influence the
"ignore" option of autofs, which is a hint for getmntent to skip over
the line.  With this change, "\151gnore" will not cuase the line to be
skipped over, but the options string will show "ignore" correctly.

Also add a test case to validate that encoded hashes get decoded
correctly.  This would be useful if/when the kernel starts escaping
hashes in /proc/mounts.
---
 misc/mntent_r.c          | 92 ++++++++++++++++++++++++----------------
 misc/tst-mntent-autofs.c |  4 +-
 2 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/misc/mntent_r.c b/misc/mntent_r.c
index b50ba0de89..ac56e40fd5 100644
--- a/misc/mntent_r.c
+++ b/misc/mntent_r.c
@@ -65,6 +65,27 @@ __endmntent (FILE *stream)
 libc_hidden_def (__endmntent)
 weak_alias (__endmntent, endmntent)
 
+static char
+parse_octal (char *s)
+{
+  char c = 0, t;
+
+  t = s[0] - '0';
+  if (t & (~0x7))
+    return 0;
+  c |= t << 6;
+
+  t = s[1] - '0';
+  if (t & (~0x7))
+    return 0;
+  c |= t << 3;
+
+  t = s[2] - '0';
+  if (t & (~0x7))
+    return 0;
+
+  return c | t;
+}
 
 /* Since the values in a line are separated by spaces, a name cannot
    contain a space.  Therefore some programs encode spaces in names
@@ -77,43 +98,42 @@ decode_name (char *buf)
   char *wp = buf;
 
   do
-    if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '4' && rp[3] == '0')
-      {
-	/* \040 is a SPACE.  */
-	*wp++ = ' ';
-	rp += 3;
-      }
-    else if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '1' && rp[3] == '1')
-      {
-	/* \011 is a TAB.  */
-	*wp++ = '\t';
-	rp += 3;
-      }
-    else if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '1' && rp[3] == '2')
-      {
-	/* \012 is a NEWLINE.  */
-	*wp++ = '\n';
-	rp += 3;
-      }
-    else if (rp[0] == '\\' && rp[1] == '\\')
-      {
-	/* We have to escape \\ to be able to represent all characters.  */
-	*wp++ = '\\';
-	rp += 1;
-      }
-    else if (rp[0] == '\\' && rp[1] == '1' && rp[2] == '3' && rp[3] == '4')
-      {
-	/* \134 is also \\.  */
-	*wp++ = '\\';
-	rp += 3;
-      }
-    else
+    {
+      if (rp[0] == '\\')
+	{
+	  char c;
+
+	  if ((c = parse_octal (&rp[1])) != 0)
+	    {
+	      *wp++ = c;
+	      rp += 3;
+	      continue;
+	    }
+	  if (rp[1] == '\\')
+	    {
+	      *wp++ = '\\';
+	      rp++;
+	      continue;
+	    }
+	}
       *wp++ = *rp;
+    }
   while (*rp++ != '\0');
 
   return buf;
 }
 
+static struct mntent *
+decode_mntent_names (struct mntent *res)
+{
+  res->mnt_fsname = decode_name (res->mnt_fsname);
+  res->mnt_dir = decode_name (res->mnt_dir);
+  res->mnt_type = decode_name (res->mnt_type);
+  res->mnt_opts = decode_name (res->mnt_opts);
+
+  return res;
+}
+
 static bool
 get_mnt_entry (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
 {
@@ -151,19 +171,19 @@ get_mnt_entry (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
   while (head[0] == '\0' || head[0] == '#');
 
   cp = __strsep (&head, " \t");
-  mp->mnt_fsname = cp != NULL ? decode_name (cp) : (char *) "";
+  mp->mnt_fsname = cp != NULL ? cp : (char *) "";
   if (head)
     head += strspn (head, " \t");
   cp = __strsep (&head, " \t");
-  mp->mnt_dir = cp != NULL ? decode_name (cp) : (char *) "";
+  mp->mnt_dir = cp != NULL ? cp : (char *) "";
   if (head)
     head += strspn (head, " \t");
   cp = __strsep (&head, " \t");
-  mp->mnt_type = cp != NULL ? decode_name (cp) : (char *) "";
+  mp->mnt_type = cp != NULL ? cp : (char *) "";
   if (head)
     head += strspn (head, " \t");
   cp = __strsep (&head, " \t");
-  mp->mnt_opts = cp != NULL ? decode_name (cp) : (char *) "";
+  mp->mnt_opts = cp != NULL ? cp : (char *) "";
   switch (head ? sscanf (head, " %d %d ", &mp->mnt_freq, &mp->mnt_passno) : 0)
     {
     case 0:
@@ -197,7 +217,7 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
 	  memset (mp, 0, sizeof (*mp));
 	else
 	  {
-	    result = mp;
+	    result = decode_mntent_names (mp);
 	    break;
 	  }
       }
diff --git a/misc/tst-mntent-autofs.c b/misc/tst-mntent-autofs.c
index e4c509f520..54b11b23ba 100644
--- a/misc/tst-mntent-autofs.c
+++ b/misc/tst-mntent-autofs.c
@@ -87,8 +87,8 @@ static struct test_case test_cases[] =
     /* These are not filtered because the string is escaped.  '\151'
        is 'i', but it is not actually decoded by the parser.  */
     { "/etc/auto.\\151gnore /mnt/auto/16 autofs \\151gnore 0 0",
-      { "/etc/auto.\\151gnore", "/mnt/auto/16", "autofs",
-        "\\151gnore", } },
+      { "/etc/auto.ignore", "/mnt/auto/16", "autofs",
+        "ignore", } },
   };
 
 static int
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PING][PATCH] getmntent: Generalize octal decoding
  2020-12-16 14:24 [PATCH] getmntent: Generalize octal decoding Siddhesh Poyarekar via Libc-alpha
@ 2020-12-22  7:48 ` Siddhesh Poyarekar
  0 siblings, 0 replies; 2+ messages in thread
From: Siddhesh Poyarekar @ 2020-12-22  7:48 UTC (permalink / raw
  To: Siddhesh Poyarekar, libc-alpha; +Cc: fweimer

On 12/16/20 7:54 PM, Siddhesh Poyarekar via Libc-alpha wrote:
> The current octal decoding looks for specific characters to decode.
> generalize this instead, allowing any arbitrary \xxx octal pattern to
> be decoded as long as it is a printable character.
> 
> The decoding is now done a bit later so that it does not influence the
> "ignore" option of autofs, which is a hint for getmntent to skip over
> the line.  With this change, "\151gnore" will not cuase the line to be
> skipped over, but the options string will show "ignore" correctly.
> 
> Also add a test case to validate that encoded hashes get decoded
> correctly.  This would be useful if/when the kernel starts escaping
> hashes in /proc/mounts.
> ---
>   misc/mntent_r.c          | 92 ++++++++++++++++++++++++----------------
>   misc/tst-mntent-autofs.c |  4 +-
>   2 files changed, 58 insertions(+), 38 deletions(-)
> 
> diff --git a/misc/mntent_r.c b/misc/mntent_r.c
> index b50ba0de89..ac56e40fd5 100644
> --- a/misc/mntent_r.c
> +++ b/misc/mntent_r.c
> @@ -65,6 +65,27 @@ __endmntent (FILE *stream)
>   libc_hidden_def (__endmntent)
>   weak_alias (__endmntent, endmntent)
>   
> +static char
> +parse_octal (char *s)
> +{
> +  char c = 0, t;
> +
> +  t = s[0] - '0';
> +  if (t & (~0x7))
> +    return 0;
> +  c |= t << 6;
> +
> +  t = s[1] - '0';
> +  if (t & (~0x7))
> +    return 0;
> +  c |= t << 3;
> +
> +  t = s[2] - '0';
> +  if (t & (~0x7))
> +    return 0;
> +
> +  return c | t;
> +}
>   
>   /* Since the values in a line are separated by spaces, a name cannot
>      contain a space.  Therefore some programs encode spaces in names
> @@ -77,43 +98,42 @@ decode_name (char *buf)
>     char *wp = buf;
>   
>     do
> -    if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '4' && rp[3] == '0')
> -      {
> -	/* \040 is a SPACE.  */
> -	*wp++ = ' ';
> -	rp += 3;
> -      }
> -    else if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '1' && rp[3] == '1')
> -      {
> -	/* \011 is a TAB.  */
> -	*wp++ = '\t';
> -	rp += 3;
> -      }
> -    else if (rp[0] == '\\' && rp[1] == '0' && rp[2] == '1' && rp[3] == '2')
> -      {
> -	/* \012 is a NEWLINE.  */
> -	*wp++ = '\n';
> -	rp += 3;
> -      }
> -    else if (rp[0] == '\\' && rp[1] == '\\')
> -      {
> -	/* We have to escape \\ to be able to represent all characters.  */
> -	*wp++ = '\\';
> -	rp += 1;
> -      }
> -    else if (rp[0] == '\\' && rp[1] == '1' && rp[2] == '3' && rp[3] == '4')
> -      {
> -	/* \134 is also \\.  */
> -	*wp++ = '\\';
> -	rp += 3;
> -      }
> -    else
> +    {
> +      if (rp[0] == '\\')
> +	{
> +	  char c;
> +
> +	  if ((c = parse_octal (&rp[1])) != 0)
> +	    {
> +	      *wp++ = c;
> +	      rp += 3;
> +	      continue;
> +	    }
> +	  if (rp[1] == '\\')
> +	    {
> +	      *wp++ = '\\';
> +	      rp++;
> +	      continue;
> +	    }
> +	}
>         *wp++ = *rp;
> +    }
>     while (*rp++ != '\0');
>   
>     return buf;
>   }
>   
> +static struct mntent *
> +decode_mntent_names (struct mntent *res)
> +{
> +  res->mnt_fsname = decode_name (res->mnt_fsname);
> +  res->mnt_dir = decode_name (res->mnt_dir);
> +  res->mnt_type = decode_name (res->mnt_type);
> +  res->mnt_opts = decode_name (res->mnt_opts);
> +
> +  return res;
> +}
> +
>   static bool
>   get_mnt_entry (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
>   {
> @@ -151,19 +171,19 @@ get_mnt_entry (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
>     while (head[0] == '\0' || head[0] == '#');
>   
>     cp = __strsep (&head, " \t");
> -  mp->mnt_fsname = cp != NULL ? decode_name (cp) : (char *) "";
> +  mp->mnt_fsname = cp != NULL ? cp : (char *) "";
>     if (head)
>       head += strspn (head, " \t");
>     cp = __strsep (&head, " \t");
> -  mp->mnt_dir = cp != NULL ? decode_name (cp) : (char *) "";
> +  mp->mnt_dir = cp != NULL ? cp : (char *) "";
>     if (head)
>       head += strspn (head, " \t");
>     cp = __strsep (&head, " \t");
> -  mp->mnt_type = cp != NULL ? decode_name (cp) : (char *) "";
> +  mp->mnt_type = cp != NULL ? cp : (char *) "";
>     if (head)
>       head += strspn (head, " \t");
>     cp = __strsep (&head, " \t");
> -  mp->mnt_opts = cp != NULL ? decode_name (cp) : (char *) "";
> +  mp->mnt_opts = cp != NULL ? cp : (char *) "";
>     switch (head ? sscanf (head, " %d %d ", &mp->mnt_freq, &mp->mnt_passno) : 0)
>       {
>       case 0:
> @@ -197,7 +217,7 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
>   	  memset (mp, 0, sizeof (*mp));
>   	else
>   	  {
> -	    result = mp;
> +	    result = decode_mntent_names (mp);
>   	    break;
>   	  }
>         }
> diff --git a/misc/tst-mntent-autofs.c b/misc/tst-mntent-autofs.c
> index e4c509f520..54b11b23ba 100644
> --- a/misc/tst-mntent-autofs.c
> +++ b/misc/tst-mntent-autofs.c
> @@ -87,8 +87,8 @@ static struct test_case test_cases[] =
>       /* These are not filtered because the string is escaped.  '\151'
>          is 'i', but it is not actually decoded by the parser.  */
>       { "/etc/auto.\\151gnore /mnt/auto/16 autofs \\151gnore 0 0",
> -      { "/etc/auto.\\151gnore", "/mnt/auto/16", "autofs",
> -        "\\151gnore", } },
> +      { "/etc/auto.ignore", "/mnt/auto/16", "autofs",
> +        "ignore", } },
>     };
>   
>   static int
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-12-22  7:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-16 14:24 [PATCH] getmntent: Generalize octal decoding Siddhesh Poyarekar via Libc-alpha
2020-12-22  7:48 ` [PING][PATCH] " Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).