xfs
[Top] [All Lists]

Re: [PATCH] xfsrestore: fix string corruption in shrink()

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] xfsrestore: fix string corruption in shrink()
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Thu, 13 Nov 2014 13:56:17 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <54650970.7010902@xxxxxxxxxxx>
References: <20141113191510.176392492@xxxxxxx> <54650970.7010902@xxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 11/13/14 13:41, Eric Sandeen wrote:
On 11/13/14 1:14 PM, Mark Tinguely wrote:
Linux strcpy() corrupts the output string when the input

Not Linux strcpy in particular; per C99:

The strcpy function copies the string pointed to by s2
(including the terminating null character) into the array
pointed to by s1. If copying takes place between objects
that overlap, the behavior is undefined.
                 ^^^^^^^^^^^^^^^^^^^^^^^^^

and output strings overlap. The shrink() function in xfsrestore
uses an overlapping strcpy() to remove special characters when
processing an interactive command line. The resultant command
will fail.

examples:
  ->  cd "AOGC exome chip core genotyping"
AOGC exome chp  core genotyping not found
  ->  cd "t t"
tt not found

Fix my manually moving the characters in the array.

Signed-off-by: Mark Tinguely<tinguely@xxxxxxx>
---
  restore/tree.c |   14 +++++++++++++-  
  1 file changed, 13 insertions(+), 1 deletion(-)

Index: b/restore/tree.c
===================================================================
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -4857,7 +4857,19 @@ distance_to_space( char *s, char *l )
  static void
  shrink( char *s, size_t cnt )
  {
-       strcpy( s, s + cnt );
+       /*
+        * Linux strcpy corrupts the string if the src and dst overlap.
+        * Manually copy the entries to the left.
+        *
+        * Since the liter array is mostly nulls, shrink is not moving

what is the "liter array?"  Ah well.  Context.  ;)

+        * the array left as intended. Does not seem to be many embedded
+        * processing characters, so leaving it for now
+        */
+       char *m = s + cnt;
+       while (*m != '\0')
+               *s++ = *m++;
+       /* NULL the last character of the string */
+       *s = '\0';
  }

Would this be any less manual?

     size_t n = strlen(s+cnt) + 1; /* 1 for terminating NULL */

     memmove(s, s + cnt, n);

because memmove is ok with overlaps.

-Eric


I thought of that but if we are doing a strlen() might as well just copy it while you are walking the string.

--Mark.

<Prev in Thread] Current Thread [Next in Thread>