[PATCH v2 0/3] sparse-checkout file handle leak fix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Here's a re-roll that fixes the use-after-free of the lock filename that
Patrick noticed. I pulled the error-checking fix into its own patch
(patch 2 here), and did Junio's suggested "goto out" as preparation in
patch 1. Patch 3 is the leak fix.

Range diff is below, but it's much harder to read than just looking at
the updated patch 3.

Thanks both for review on round 1.

  [1/3]: sparse-checkout: consolidate cleanup when writing patterns
  [2/3]: sparse-checkout: check commit_lock_file when writing patterns
  [3/3]: sparse-checkout: use fdopen_lock_file() instead of xfdopen()

 builtin/sparse-checkout.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

-:  ---------- > 1:  03a80d3748 sparse-checkout: consolidate cleanup when writing patterns
-:  ---------- > 2:  20628aa350 sparse-checkout: check commit_lock_file when writing patterns
1:  b4700ba1a9 ! 3:  fe41f9f02b sparse-checkout: use fdopen_lock_file() instead of xfdopen()
    @@ Commit message
           - we no longer need to call fflush(); committing the lock-file
             auto-closes it, which will now do the flush for us. As a bonus, this
             will actually check that the flush was successful before renaming
    -        the file into place. Let's likewise report when committing the lock
    -        fails (rather than quietly returning success from the command).
    +        the file into place.
     
           - we can get rid of the local "fd" variable, since we never look at it
             ourselves now
    @@ builtin/sparse-checkout.c: static int write_patterns_and_update(struct pattern_l
     -	fd = hold_lock_file_for_update(&lk, sparse_filename,
     -				      LOCK_DIE_ON_ERROR);
     +	hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR);
    - 	free(sparse_filename);
      
      	result = update_working_directory(pl);
    + 	if (result) {
     @@ builtin/sparse-checkout.c: static int write_patterns_and_update(struct pattern_list *pl)
    - 		return result;
    + 		goto out;
      	}
      
     -	fp = xfdopen(fd, "w");
    @@ builtin/sparse-checkout.c: static int write_patterns_and_update(struct pattern_l
      		write_patterns_to_file(fp, pl);
      
     -	fflush(fp);
    --	commit_lock_file(&lk);
    -+	if (commit_lock_file(&lk))
    -+		die_errno(_("unable to write %s"), get_locked_file_path(&lk));
    - 
    - 	clear_pattern_list(pl);
    + 	if (commit_lock_file(&lk))
    + 		die_errno(_("unable to write %s"), sparse_filename);
      





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux