Bug 239685 - cap_fileargs(3) is not robust against fd limits
Summary: cap_fileargs(3) is not robust against fd limits
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Mariusz Zaborski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-07 03:21 UTC by Mark Johnston
Modified: 2019-08-13 18:43 UTC (History)
3 users (show)

See Also:


Attachments
git(1) diff for some fixes (3.07 KB, patch)
2019-08-08 18:55 UTC, Kyle Evans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Johnston freebsd_committer 2019-08-07 03:21:59 UTC
See the commit log message for r350671.  I can't seem to reproduce the hang by hand, but exceeding the limit does cause readelf to die with SIGPIPE, which is not right.
Comment 1 Mark Johnston freebsd_committer 2019-08-07 16:31:23 UTC
Kyle did some debugging and found that the issue comes from uipc_externalize() returning an error when it fails to allocate descriptors in the receiving process' fd table.  I believe this results in an EMSGSIZE error from recvmsg().
Comment 2 Kyle Evans freebsd_committer 2019-08-07 19:46:46 UTC
(In reply to Mark Johnston from comment #1)

I think this might have been a red herring / faulty debugging somewhere -- my distilled test cases all seem to be doing the correct thing.
Comment 3 Mark Johnston freebsd_committer 2019-08-07 20:06:20 UTC
(In reply to Kyle Evans from comment #2)
"Correct" meaning that fileargs_open() fails with EMFILE?
Comment 4 Kyle Evans freebsd_committer 2019-08-08 18:55:00 UTC
Created attachment 206375 [details]
git(1) diff for some fixes

(In reply to Mark Johnston from comment #3)

In that context, I was trying to rewrite a smaller test case that reproduces the results -- that was a mistake.

There seem to be at least a couple of different problems here:

- fileargs_add_cache can fail, but not communicate that failure to its parent
- service_message should check nvlout for error state after taking a trip through service->s_command -- if nvlout happens to be an error state, cap_send_nvlist cannot succeed.

The SIGPIPE failure seems to originate from fileargs_add_cache hitting EMFILE at nvlist_add_nvlist(nvlout, fname, new), which then fails to travel because nvlout's in an error state.

Attached patch tries to clean some of this up, but doesn't do a very good job I don't think. Ultimately service_message should be catching any last-minute errors and trying to recover gracefully so we don't leave the other end in suspense, but the rest of the patch attempts to detect errors earlier and do something sane...