Bug 250872

Summary: objcopy (elfcopy, elftoolchain) in-place operation emits output to wrong filesystem
Product: Base System Reporter: Conrad Meyer <cem>
Component: binAssignee: Dimitry Andric <dim>
Status: Closed FIXED    
Severity: Affects Only Me CC: dim, emaste, swills
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   

Description Conrad Meyer freebsd_committer freebsd_triage 2020-11-05 02:04:53 UTC
objcopy can fail with 'write failed: No space left on device' when working on a file in a filesystem with tons of free space.  (I.e., no explicit destination file, dst == src.)

This is because create_tempfile() is completely ignorant of any destination goal and writes everything to /tmp.  Then, for files outside of /tmp, copy_from_tempfile() cannot rely on rename(2) putting the output in place of the input file and has to instead do a full read-copy-write from /tmp.

For "temporary" files which are really the work-in-progress output file, they should be emitted to the destination directory and renamed over the top of the destination file.  I'd suggest:

1. Add an optional destination path parameter to create_tempfile.  create_file() passes src in 'dst == NULL' case near top of function, and possibly for other consumers in the same file if they are potentially the final output file.

2. create_tempfile constructs a path adjacent to 'src' (that doesn't exist already) and opens it, if src is provided.  Otherwise, it uses its existing logic to construct a temporary file in /tmp.

3. The rest of the logic in create_file / copy_from_tempfile should handle this correctly already.
Comment 1 Dimitry Andric freebsd_committer freebsd_triage 2020-11-06 17:38:41 UTC
I submitted https://sourceforge.net/p/elftoolchain/tickets/597/ upstream, because I would rather not introduce more gratuitous differences between our elftoolchain and upstream. If they don't respond soonish, I will commit that diff into our tree instead.
Comment 2 Conrad Meyer freebsd_committer freebsd_triage 2020-11-06 18:03:05 UTC
Thanks!
Comment 3 commit-hook freebsd_committer freebsd_triage 2020-11-18 18:41:17 UTC
A commit references this bug:

Author: dim
Date: Wed Nov 18 18:40:59 UTC 2020
New revision: 367809
URL: https://svnweb.freebsd.org/changeset/base/367809

Log:
  When elftoolchain's objcopy (or strip) is rewriting a file in-place,
  make it create the temporary file in the same directory as the source
  file by default, instead of always using $TMPDIR or /tmp. If creating
  that file fails because the directory is not writable, also fallback to
  $TMPDIR or /tmp.

  This has also been submitted upstream as:
  https://sourceforge.net/p/elftoolchain/tickets/597/

  Reported by:	cem
  PR:		250872
  MFC after:	2 weeks

Changes:
  head/contrib/elftoolchain/elfcopy/archive.c
  head/contrib/elftoolchain/elfcopy/elfcopy.h
  head/contrib/elftoolchain/elfcopy/main.c
Comment 4 Conrad Meyer freebsd_committer freebsd_triage 2020-11-18 19:05:11 UTC
Thanks again :-).
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-12-02 21:45:42 UTC
A commit references this bug:

Author: dim
Date: Wed Dec  2 21:44:43 UTC 2020
New revision: 368286
URL: https://svnweb.freebsd.org/changeset/base/368286

Log:
  MFC r367809:

  When elftoolchain's objcopy (or strip) is rewriting a file in-place,
  make it create the temporary file in the same directory as the source
  file by default, instead of always using $TMPDIR or /tmp. If creating
  that file fails because the directory is not writable, also fallback to
  $TMPDIR or /tmp.

  This has also been submitted upstream as:
  https://sourceforge.net/p/elftoolchain/tickets/597/

  Reported by:	cem
  PR:		250872

Changes:
_U  stable/11/
  stable/11/contrib/elftoolchain/elfcopy/archive.c
  stable/11/contrib/elftoolchain/elfcopy/elfcopy.h
  stable/11/contrib/elftoolchain/elfcopy/main.c
_U  stable/12/
  stable/12/contrib/elftoolchain/elfcopy/archive.c
  stable/12/contrib/elftoolchain/elfcopy/elfcopy.h
  stable/12/contrib/elftoolchain/elfcopy/main.c