Bug 266264 - file(1): incorrect output when file name is read from standard input
Summary: file(1): incorrect output when file name is read from standard input
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Xin LI
URL:
Keywords:
: 265982 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-09-07 05:51 UTC by Yuri Victorovich
Modified: 2022-09-30 07:52 UTC (History)
5 users (show)

See Also:


Attachments
Patch for return from realloc (415 bytes, patch)
2022-09-23 21:14 UTC, Adriaan de Groot
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2022-09-07 05:51:22 UTC
This command produces incorrect output on 13-STABLE:
> $ echo /bin/ls | file -f -
> /:       ELF 64-bit LSB pie executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 13.1 (1301506), FreeBSD-style, stripped

The correct output is:
> $ echo /bin/ls | file -f -
> /bin/ls:       ELF 64-bit LSB pie executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 13.1 (1301506), FreeBSD-style, stripped


I found this while investigating why doesn't 'make stage-qa' print dependency recommendations for ports, and narrowed it to this problem.
Comment 1 Yuri Victorovich freebsd_committer freebsd_triage 2022-09-07 06:00:52 UTC
Xin,

On Jul 3 contrib/file was upgraded to 5.42
Could this upgrade have caused this problem?


Yuri
Comment 2 Xin LI freebsd_committer freebsd_triage 2022-09-07 06:21:40 UTC
You are right, the fix was https://github.com/file/file/commit/19bf47777d0002ee884467e45e6ace702e40a4c1 .
Comment 3 Xin LI freebsd_committer freebsd_triage 2022-09-07 06:28:44 UTC
Upstream bug: https://bugs.astron.com/view.php?id=358
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-09-07 06:33:13 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bced4d8b3e99efd46b5496c83e11755b25913c90

commit bced4d8b3e99efd46b5496c83e11755b25913c90
Merge: c65e42dbde41 5457a3f25818
Author:     Xin LI <delphij@FreeBSD.org>
AuthorDate: 2022-09-07 06:31:20 +0000
Commit:     Xin LI <delphij@FreeBSD.org>
CommitDate: 2022-09-07 06:31:20 +0000

    MFV: cherry-pick "PR/358: Fix width for -f - (jpalus)"

    MFC after:      1 week
    PR:             bin/266264

 contrib/file/src/file.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2022-09-07 06:50:16 UTC
*** Bug 265982 has been marked as a duplicate of this bug. ***
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-09-15 05:32:03 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=df4d33ecaac46330250472c096714f717950879c

commit df4d33ecaac46330250472c096714f717950879c
Author:     Xin LI <delphij@FreeBSD.org>
AuthorDate: 2022-09-07 06:31:20 +0000
Commit:     Xin LI <delphij@FreeBSD.org>
CommitDate: 2022-09-15 05:31:13 +0000

    MFV: cherry-pick "PR/358: Fix width for -f - (jpalus)"

    PR:             bin/266264
    (cherry picked from commit bced4d8b3e99efd46b5496c83e11755b25913c90)

 contrib/file/src/file.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-09-15 05:35:04 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=59146889b1b1499ccf1769261cf8c98a85ce33db

commit 59146889b1b1499ccf1769261cf8c98a85ce33db
Author:     Xin LI <delphij@FreeBSD.org>
AuthorDate: 2022-09-07 06:31:20 +0000
Commit:     Xin LI <delphij@FreeBSD.org>
CommitDate: 2022-09-15 05:34:06 +0000

    MFV: cherry-pick "PR/358: Fix width for -f - (jpalus)"

    MFC after:      1 week
    PR:             bin/266264

    (cherry picked from commit bced4d8b3e99efd46b5496c83e11755b25913c90)

 contrib/file/src/file.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)
Comment 8 Adriaan de Groot freebsd_committer freebsd_triage 2022-09-23 21:06:40 UTC
The fix committed to 13-STABLE is bad. Sending more than 100 filenames to file(1) now causes a segmentation fault or bus error.

How to reproduce on 13-STABLE:
- get a list of more than 100 files (e.g. `ls /sbin` -- I have 150 files in /sbin/)
- send 100 files of that list to file(1) like `ls /sbin | head -100 | file -f -` and notice this works, all 100 are identified
- send 101 files of that list to file(1) like `ls /sbin | head -101 | file -f -` and notice it crashes
Comment 9 Adriaan de Groot freebsd_committer freebsd_triage 2022-09-23 21:14:31 UTC
Created attachment 236778 [details]
Patch for return from realloc
Comment 10 Adriaan de Groot freebsd_committer freebsd_triage 2022-09-23 21:16:14 UTC
The problem is in this code, added in df4d33ecaac46330250472c096714f717950879c:

+               if (fi >= fimax) {
+                       fimax += 100;
+                       char **nf = realloc(flist, fimax * sizeof(*flist));
+                       if (nf == NULL)
+                               goto out;
+               }

after the `realloc()` call, `flist` is freed, and the newly-allocated memory is not used. The new pointer `nf` needs to be assigned to `flist` before continuing.
Comment 11 lampa 2022-09-26 07:31:24 UTC
It also breaks portupgrade, but proposed fix is not sufficient:

portupgrade -a     (after applied patch, make and make install)
....
--->  Backing up the old version
pkg: unknown format pkg, using the default
--->  Uninstalling the old version
[Reading data from pkg(8) ... - 816 packages found - done]
--->  Deinstalling 'perl5-5.32.1_1'
<<<<< STOPS HERE

ps ax

20108  0  I+       0:00,31 /usr/local/bin/ruby30 /usr/local/sbin/pkg_deinstall
20193  0  I+       0:00,00 /usr/bin/file -bnf-

Revert to previous version of file.c helps.
Comment 12 Xin LI freebsd_committer freebsd_triage 2022-09-26 17:21:58 UTC
Does this still happen after file 5.43?  (If cherry-picking locally, use: git cherry-pick -m 1 a2dfb7224ec9).
Comment 13 Adriaan de Groot freebsd_committer freebsd_triage 2022-09-26 18:23:18 UTC
On 13-STABLE, I can see 0be784ff6c4728aa3578d7c8741d20a440a81395 which contains at least the `flist = nf;` fix for my problem, so I'll say "no, this problem does not happen after 5.43".
Comment 14 Xin LI freebsd_committer freebsd_triage 2022-09-28 05:23:35 UTC
MFC'ed file 5.43 to stable/13 and stable/12.
Comment 15 lampa 2022-09-30 07:52:40 UTC
(In reply to Xin LI from comment #12)

Yes, as I've written, I have applied patch by hand. Also file-5.43 don't fix the portupgrade problem.

[Reading data from pkg(8) ... - 817 packages found - done]
--->  Deinstalling 'expat-2.4.8'
<<<< STALL


8002  3  S+        0:04,32 ruby30: portupgrade: [1/10] expat-2.4.8 (ruby30)
64009  3  S+        0:00,29 /usr/local/bin/ruby30 /usr/local/sbin/pkg_deinstall
64094  3  S+        0:00,00 /usr/bin/file -bnf-

file -v
file-5.43
magic file from /usr/share/misc/magic

It looks that pkg_deinstall stops at the first file processed by file. It expects output from the 'file', but doesn't get any, since the processing of files is now delayed after end of input:

        pipe.puts(file)

        filetype = pipe.gets

I think, that this change from synchronous processing to asynchronous processing breaks more applications then portupgrade, please revert it.