All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Mark Tinguely <tinguely@sgi.com>, xfs@oss.sgi.com
Subject: Re: [PATCH] xfsrestore: fix string corruption in shrink()
Date: Thu, 13 Nov 2014 13:41:36 -0600	[thread overview]
Message-ID: <54650970.7010902@sandeen.net> (raw)
In-Reply-To: <20141113191510.176392492@sgi.com>

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@sgi.com>
> ---
>  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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-11-13 19:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-13 19:14 [PATCH] xfsrestore: fix string corruption in shrink() Mark Tinguely
2014-11-13 19:41 ` Eric Sandeen [this message]
2014-11-13 19:56   ` Mark Tinguely
2014-11-13 20:45     ` Eric Sandeen
2014-11-13 21:57       ` Mark Tinguely
2014-11-13 21:50     ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54650970.7010902@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=tinguely@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.