Bug 255589 - strip(1) leaves empty file when applied to unstrippable file
Summary: strip(1) leaves empty file when applied to unstrippable file
Status: Closed Overcome By Events
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: needs-patch
Depends on:
Blocks:
 
Reported: 2021-05-04 10:24 UTC by Adriaan de Groot
Modified: 2022-03-28 21:06 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adriaan de Groot freebsd_committer freebsd_triage 2021-05-04 10:24:24 UTC
When strip(1) is applied to a file that it can't strip, it leaves behind an empty file alongside the thing it was stripping.

To reproduce:

```
    mkdir /tmp/example-for-strip
    echo "bogus" > /tmp/example-for-strip/mytextfile
    strip /tmp/example-for-strip/mytextfile
```

Output from strip is
```
    strip: file format not recognized
```

The directory /tmp/example-for-strip now contains two files: the mytextfile -- that's intended -- and a 0-byte ecp.<random> file, which is not intended.

This affects ports builds where sometimes strip is applied to scripts -- that's nominally a build-system problem for the port in question, but strip(1) shouldn't be leaving spare files around anyway.
Comment 1 Adriaan de Groot freebsd_committer freebsd_triage 2021-05-04 10:50:38 UTC
I'm *fairly* sure this was introduced in upstream https://sourceforge.net/p/elftoolchain/code/3919/ and that it needs cleanup to remove the tempfile on the error-exit path with unrecognized input, something like this:

```
        switch (elf_kind(ecp->ein)) {
        case ELF_K_NONE:
                if (tempfile != NULL) {
                        if (unlink(tempfile) < 0)
                                err(EXIT_FAILURE, "unlink %s failed", tempfile);
                        free(tempfile);
                }
                errx(EXIT_FAILURE, "file format not recognized");
```

Adding crees@ as upstream-involved.

In the meantime, ports are getting cruft like `${RM} ${STAGEDIR}${LOCALBASE}/bin/ecp.*` to clean up after strip(1).

Note that you don't need to use a full path for the strip command; soemthing like `strip *.png` will bail out on the first png file and leave a single `ecp.<random>` file in the current directory.
Comment 2 Adriaan de Groot freebsd_committer freebsd_triage 2021-05-04 11:06:52 UTC
As an example of what happens in a ports build: as part of the install step, for whatever silly reason, a non-ELF file is stripped:


strip /wrkdirs/usr/ports/x11/lumina-core/work/stage/usr/local/share/lumina-desktop/menu-scripts/ls.json.sh
strip: file format not recognized


and now there's an orphan file

Error: Orphaned: share/lumina-desktop/menu-scripts/ecp.bN32GnFD
Comment 3 Chris Rees freebsd_committer freebsd_triage 2021-05-04 11:39:26 UTC
Yes, I introduced this fix in 5ac70383, but somehow I didn't manage to get it into 13.0-RELEASE, which is actually a major pain.

It was always a problem, but didn't appear until the default directory for strip(1) temporary files was changed to '.' from /tmp in 1e4896b1.

Portmgr, I'm really sorry, but I think we have no alternative but to put an OSVERSION check and then find ${WRKSRC} -type f -name 'ecp.*' -delete on the do-strip target in bsd.port.mk or wherever, otherwise the tree is going to be totally littered with ad-hoc fixes that only apply to 13.0-R.

Alternatively, we could (ab)use the releng/13.0 branch...
Comment 4 Adriaan de Groot freebsd_committer freebsd_triage 2021-05-04 12:16:00 UTC
Thanks Chris for the quick response. Yeah, I've got 5 .. maybe 8 ports commits lined up (thanks to git, just locally) that all are variations on

sysutils/luckybackup: workaround strip(1) leaving behind empty files

With my ports hat on, they're pretty imporant, because otherwise I can't tell the difference between "plist failure because of strip(1)" and "plist failure for some other legitimate reason"; I'll keep them local for the time being.
Comment 5 Antoine Brodin freebsd_committer freebsd_triage 2021-05-04 12:43:32 UTC
why not an ERRATA NOTICE in base?
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2021-05-04 13:12:31 UTC
If anyone could mail secteam@ with some reference to the commit and ideally a filled out copy of the erratum notice template (https://www.freebsd.org/security/errata-template.txt), we can include it in the next batch.
Comment 7 Felix Palmen freebsd_committer freebsd_triage 2021-07-10 09:39:29 UTC
As this happens to me now while upgrading emulators/emu64, is the best course of action to just ignore the resulting plist-errors until this is fixed?
Comment 8 Adriaan de Groot freebsd_committer freebsd_triage 2021-07-11 16:15:13 UTC
@felix What I do now is run `poudriere -t` for a single port, and take note if it fails because of strip(1) bugs .. and then drop the `-t` to get a package for local consumption. I don't think the cluster runs testport, so if doesn't really affect packages elsewhere and it's just annoying when building individual ports with `-t`.

(For the record, I don't feel qualified to deal with errata or secteam wrt. base bin/)
Comment 9 Mateusz Piotrowski freebsd_committer freebsd_triage 2021-10-09 22:12:36 UTC
I've just hit this bug while packaging devel/subversion. Did anyone have time to contact the secteam?
Comment 10 Baptiste Daroussin freebsd_committer freebsd_triage 2021-11-08 13:50:26 UTC
This is not a portmgr bug
Comment 11 Adriaan de Groot freebsd_committer freebsd_triage 2022-03-28 21:06:08 UTC
This still applies to 12.3, and to 13.0 (both supported releases as of this comment). It does not apply to 13-STABLE, or upcoming 13.1 release. It's annoying to ports, but there is a workaround, and overall my impression is that this is a "meh, whatevs" kind of thing which might have been errata'ed in 2021 but just isn't worth having on my bug list anymore.