Bug 235121 - shells/rssh: rssh crashes on invocation due to new basename(3) POSIX behaviour
Summary: shells/rssh: rssh crashes on invocation due to new basename(3) POSIX behaviour
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-22 07:40 UTC by topical
Modified: 2019-02-16 20:36 UTC (History)
2 users (show)

See Also:
jharris: maintainer-feedback+


Attachments
basename(3): avoid unnecessary writing to passed string (316 bytes, patch)
2019-01-22 07:41 UTC, topical
no flags Details | Diff
rssh: pass string copy to basename(3) so it can safely modify it (709 bytes, patch)
2019-01-22 07:43 UTC, topical
no flags Details | Diff
comply POSIX standard when using basename, part 1 (766 bytes, patch)
2019-02-05 11:46 UTC, topical
no flags Details | Diff
comply POSIX standard when using basename, part 2 (869 bytes, patch)
2019-02-05 11:46 UTC, topical
no flags Details | Diff
comply POSIX standard when using basename, part 3 (370 bytes, patch)
2019-02-05 11:47 UTC, topical
no flags Details | Diff
comply POSIX standard when using basename, part 4 (688 bytes, patch)
2019-02-05 11:47 UTC, topical
no flags Details | Diff
shar patch - supersedes all others (3.06 KB, patch)
2019-02-16 20:36 UTC, jharris
jharris: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description topical 2019-01-22 07:40:01 UTC
In FreeBSD 12.0, basename(3) was changed to be POSIX compliant. This implies that it can possibly write to the passed string.

rssh passes a const string, so it always crashes on invocation.

There are 2 solutions:

  * modify basename(3) so it only modifies the string if it has to
  * modify rssh, making a copy of the string first and pass it instead

The first solution may help other ports as well but I know that it will take some time until this change will be public. Meanwhile we could fix at least this single port by making it more POSIX compliant.
Comment 1 topical 2019-01-22 07:41:19 UTC
Created attachment 201321 [details]
basename(3): avoid unnecessary writing to passed string
Comment 2 topical 2019-01-22 07:43:00 UTC
Created attachment 201322 [details]
rssh: pass string copy to basename(3) so it can safely modify it
Comment 3 jharris 2019-02-03 05:25:21 UTC
Comment on attachment 201322 [details]
rssh: pass string copy to basename(3) so it can safely modify it

This patch looks good.  Approved.
Comment 4 jharris 2019-02-03 05:28:05 UTC
Comment on attachment 201321 [details]
basename(3): avoid unnecessary writing to passed string

There is no basename.c in the extracted work/rssh-2.3.4 directory, and we can't patch basename.c for the kernel, etc., so this won't work.  Not approved.
Comment 5 jharris 2019-02-03 05:35:58 UTC
topical, I think you'll want to add your same fix from util.c before all invocations of basename(3):

%grep -n basename *.c
log.c:96:       ident = strdup(basename((char*)name));
rssh_chroot_helper.c:162:       progname = strdup(log_make_ident(basename(argv[0])));
util.c:166:     char    *prog;          /* basename of cmd */
util.c:189:             prog = basename(cmd);


So also in log.c and rssh_chroot_helper.c, right?

Thanks!
Comment 6 Koichiro Iwao freebsd_committer 2019-02-05 01:55:21 UTC
Tell me when final fix is done and which is the final decision. Then I'll commit it to ports tree.
Comment 7 topical 2019-02-05 11:46:27 UTC
Created attachment 201752 [details]
comply POSIX standard when using basename, part 1
Comment 8 topical 2019-02-05 11:46:49 UTC
Created attachment 201753 [details]
comply POSIX standard when using basename, part 2
Comment 9 topical 2019-02-05 11:47:13 UTC
Created attachment 201754 [details]
comply POSIX standard when using basename, part 3
Comment 10 topical 2019-02-05 11:47:31 UTC
Created attachment 201755 [details]
comply POSIX standard when using basename, part 4
Comment 11 topical 2019-02-05 11:49:17 UTC
jharris, you are right - there are multiple invocations of basename. I tried to fix them all and created a patch set.
Comment 12 jharris 2019-02-16 20:36:12 UTC
Created attachment 202073 [details]
shar patch - supersedes all others

This final sharfile of 3 patches follows the original coding style in util.c and _attempts_ to be as clear as possible.  It supersedes all earlier patches on this PR.

topical, thanks for pointing out the problem with the port on FreeBSD 12 and your initial patches.

Tested on: 12.0-RELEASE-p3 GENERIC  amd64