Subject: Re: bug#11761: Slight bug in split :-)

From: Jim Meyering <[email-masked]>
Date: Fri, 22 Jun 2012 09:56:24 +0200

Pádraig Brady wrote:
...
> diff --git a/src/split.c b/src/split.c
> index 53ee271..3e3313a 100644
> --- a/src/split.c
> +++ b/src/split.c
> @@ -92,6 +92,9 @@ static char const *additional_suffix;
>  /* Name of input file.  May be "-".  */
>  static char *infile;
>
> +/* stat buf for input file.  */
> +static struct stat in_stat_buf;
> +
>  /* Descriptor on which output file is open.  */
>  static int output_desc = -1;
>
> @@ -362,6 +365,17 @@ create (const char *name)
>      {
>        if (verbose)
>          fprintf (stdout, _("creating file %s\n"), quote (name));
> +
> +      struct stat out_stat_buf;
> +      if (stat (name, &out_stat_buf) == 0)
> +        {
> +          if (SAME_INODE (in_stat_buf, out_stat_buf))
> +            error (EXIT_FAILURE, 0, _("%s would overwrite input. Aborting."),
> +                   quote (name));
> +        }
> +      else if (errno != ENOENT)
> +        error (EXIT_FAILURE, errno, _("cannot stat %s"), quote (name));
> +
>        return open (name, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                     (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH))
>      }

Hi Pádraig,

Thanks for taking this on.
That introduces a minor TOCTOU race.
It would probably never matter in practice,
but who knows... if we can avoid it, why not?
What do you think about something like this?

    int fd = open (name, (... as above, but without O_TRUNC...)...
    if (fd < 0)
      return fd;
    if ( ! fstat (fd, &out_stat_buf))
      error (EXIT_FAILURE, errno, _("failed to fstat %s"), quote (name));
    if (SAME_INODE (in_stat_buf, out_stat_buf))
      error (EXIT_FAILURE, 0, _("%s would overwrite input. Aborting."),
             quote (name));
    if ( ! ftruncate (fd, 0))
      error ...
    return fd;

The above might even be a tiny bit faster for long names,
since it resolves each name only once.