xfs
[Top] [All Lists]

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

To: Mark Tinguely <tinguely@xxxxxxx>, xfs@xxxxxxxxxxx
Subject: Re: [PATCH] xfsrestore: fix string corruption in shrink()
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 13 Nov 2014 13:41:36 -0600
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20141113191510.176392492@xxxxxxx>
References: <20141113191510.176392492@xxxxxxx>
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


>  static int
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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