Bug 277060 - pax(1) hangs when copying directories with a trailing slash
Summary: pax(1) hangs when copying directories with a trailing slash
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Warner Losh
URL:
Keywords: patch-ready
Depends on:
Blocks:
 
Reported: 2024-02-15 00:59 UTC by c433li
Modified: 2024-06-22 12:31 UTC (History)
2 users (show)

See Also:
martymac: maintainer-feedback?


Attachments
Proposed patch (525 bytes, patch)
2024-06-07 17:01 UTC, Ganael LAPLANCHE
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description c433li 2024-02-15 00:59:03 UTC
The issue is best illustrated with a demo first:

----
# mkdir demo
# cd demo
# echo '/usr/local/' | pax -rw .
(hangs forever, interrupt with Ctrl-C)
# echo '/usr/local/' | pax -rw .
(this time it completes successfully)
----

In other words, when `pax(1)` is operating under copy mode (the forth synopsis form):

----
pax -r -w [file ...] directory
----

If *all* the following conditions are met, it will hang forever:
1. the `file` operand is not specified; and
2. its has its standard input piped; and
3. the (piped) standard input contains a line that is directory (as the conceptual equivalence of `file` operand); and
4. the `file`(which is a directory) contains at least two components ('*/*/'); and
5. the destination directory does not contain all the parent components of `file`; and
6. the directory is specified with a slash (`/`) at the end.
Comment 1 Ganael LAPLANCHE freebsd_committer freebsd_triage 2024-06-07 17:00:19 UTC
Hello,

I am facing the same bug too :

$ mkdir -p /tmp/src/foo/bar
$ rm -rf /tmp/dst ; mkdir -p /tmp/dst
$ cd /tmp/src
$ echo 'foo/bar/' | /bin/pax -r -w -d -pe "/tmp/dst"
<looping infinitely>

Here, pax(1) infinitely deletes and re-creates /tmp/dst/foo/bar/.

The problem is that chk_path() (bin/pax/file_subs.c), called from node_creat() also creates the leaf directory when a trailing '/' appears in the directory name to create. When the execution goes back from chk_path() to node_creat(), the function still cannot create the leaf directory (it has been created by chk_path()), so it unlinks it and calls node_creat() again. The function re-creates it, and so on...

A possible fix is to make node_creat() detect trailing slashes so it does NOT create a leaf directory but only intermediate ones. I've added a simple check in that way in the attached patch.

Best regards,

Ganael.
Comment 2 Ganael LAPLANCHE freebsd_committer freebsd_triage 2024-06-07 17:01:10 UTC
Created attachment 251272 [details]
Proposed patch
Comment 3 Warner Losh freebsd_committer freebsd_triage 2024-06-21 15:40:58 UTC
Why
+		if ((spt == NULL) || (*(spt + 1) == '\0'))
and not
+		if ((spt == NULL) || *spt == '/'))

???
Comment 4 Warner Losh freebsd_committer freebsd_triage 2024-06-21 15:46:26 UTC
(In reply to Warner Losh from comment #3)
Oh, wait, that won't work so well. Never mind.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-06-21 16:44:06 UTC
A commit in branch main references this bug:

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

commit 681fd2bed8eaba88693867ba928a1c03a5b152cc
Author:     Ganael Laplanche <ganael.laplanche@martymac.org>
AuthorDate: 2024-06-21 16:39:09 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2024-06-21 16:39:09 +0000

    pax: Terminate loop for empty directory names

    Pax can sometimes loop forever. For example:

    $ mkdir -p /tmp/src/foo/bar
    $ rm -rf /tmp/dst ; mkdir -p /tmp/dst
    $ cd /tmp/src
    $ echo 'foo/bar/' | /bin/pax -r -w -d -pe "/tmp/dst"
    <looping infinitely>

    Here, pax(1) infinitely deletes and re-creates /tmp/dst/foo/bar/.

    The problem is that chk_path() (bin/pax/file_subs.c), called from
    node_creat() also creates the leaf directory when a trailing '/' appears
    in the directory name to create. When the execution goes back from
    chk_path() to node_creat(), the function still cannot create the leaf
    directory (it has been created by chk_path()), so it unlinks it and
    calls node_creat() again. The function re-creates it, and so on...

    In node_creat() detect trailing slashes and not create a leaf directory,
    but only intermediate ones.

    PR: 277060
    Reviewed by: imp

 bin/pax/file_subs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Comment 6 Ganael LAPLANCHE freebsd_committer freebsd_triage 2024-06-21 16:45:42 UTC
Hi Warner,

Thanks a lot for having taken care of this!

Cheers,

Ganael.
Comment 7 Mark Linimon freebsd_committer freebsd_triage 2024-06-22 04:27:18 UTC
^Triage: assign to committer.
Comment 8 Ganael LAPLANCHE freebsd_committer freebsd_triage 2024-06-22 12:31:25 UTC
Hello c433li,

Thanks for your report.

The issue has been patched. Don't hesitate to re-open that PR if you think something is missing.

Best regards,

Ganael.