Page MenuHomeFreeBSD
Differential D6546

exec: Provide execpath in imgp for the process_exec hook.
ClosedPublic
Actions

Authored by bdrewery on May 24 2016, 9:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jan 7, 7:53 PM
Unknown Object (File)
Tue, Jan 7, 2:39 PM
Unknown Object (File)
Wed, Jan 1, 11:38 AM
Unknown Object (File)
Tue, Dec 17, 5:24 PM
Unknown Object (File)
Dec 2 2024, 2:56 AM
Unknown Object (File)
Dec 2 2024, 2:56 AM
Unknown Object (File)
Dec 2 2024, 2:56 AM
Unknown Object (File)
Dec 2 2024, 2:56 AM
View All Files
Subscribers
imp

Details

Reviewers
kib
Commits
rS300793: exec: Provide execpath in imgp for the process_exec hook.
Summary

This was previously set after the hook and only if auxargs were present.
Now always provide it if possible.

MFC after: 2 weeks
Sponsored by: EMC / Isilon Storage Division

Test Plan

Used it in filemon, rather than vn_fullpath(9), and confirmed that the value
was correct.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3974
Build 4017: arc lint + arc unit

Event Timeline

bdrewery updated this revision to Diff 16821.May 24 2016, 9:32 PM
bdrewery retitled this revision from to exec: Provide execpath in imgp for the process_exec hook..
bdrewery updated this object.
bdrewery edited the test plan for this revision. (Show Details)
bdrewery added a reviewer: kib.
Herald added a subscriber: imp. · View Herald TranscriptMay 24 2016, 9:32 PM
bdrewery planned changes to this revision.May 24 2016, 9:32 PM
bdrewery added a parent revision: D6544: exec: Add credential change information into imgp for process_exec hook..

I just realized I didn't drop the vnode lock before calling vn_fullpath.

bdrewery updated this revision to Diff 16822.May 24 2016, 9:40 PM

Unlock before calling vn_fullpath(9).

kib edited edge metadata.May 26 2016, 6:00 AM

It is too early to calculate file path before interpreter is parsed.

bdrewery added a comment.May 26 2016, 3:52 PM
In D6546#138831, @kib wrote:

It is too early to calculate file path before interpreter is parsed.

Can we compromise with this and recalculate after the interpreter is known, if one is used? At least for my use case I don't mind getting the script's path as execpath in the hook instead of the interpreter path.

kib added a comment.May 26 2016, 4:04 PM
In D6546#138981, @bdrewery wrote:
In D6546#138831, @kib wrote:

It is too early to calculate file path before interpreter is parsed.

Can we compromise with this and recalculate after the interpreter is known, if one is used? At least for my use case I don't mind getting the script's path as execpath in the hook instead of the interpreter path.

I do not mind but I would prefer that

  1. it was not called execpath in imgp, but e.g. scriptpath, with execpath preserving the current semantic
  2. might be, only fill scriptpath if some global flag is set, indicating that there is consumer ? vn_fullpath is expensive
  3. for the same reason, reuse scriptpath to set execpath if executable is not interpreted

2 is optional.

bdrewery abandoned this revision.May 26 2016, 4:09 PM

I'm going to put this off for now and explore the namecache fix discussed a few weeks ago briefly on IRC.

bdrewery added a comment.May 26 2016, 5:12 PM

For interpreted it will go back and go over the execpath code I have. Couldn't I just cleanup execpath before the goto interpret and the code I have mostly works? I assume the process_exec hook is not called until the actual interpreter is handled in the activator.

bdrewery added a comment.May 26 2016, 5:21 PM
In D6546#139026, @bdrewery wrote:

For interpreted it will go back and go over the execpath code I have. Couldn't I just cleanup execpath before the goto interpret and the code I have mostly works? I assume the process_exec hook is not called until the actual interpreter is handled in the activator.

Yeah, the process_exec hook is called from exec_new_vmspace, after interpreter is looped and set into imgp->vp. So I should just be able to recalculate the execpath in this case. I think a downside is that vn_fullpath may be called on the first pass and its result thrown away.

bdrewery updated this revision to Diff 16924.May 26 2016, 5:26 PM
bdrewery edited edge metadata.
  • Free freepath for interpreter cleanup
  • Tested with running a script and filemon's use of imgp->execpath showed the interpreter fine here, and NOT the script name.
bdrewery updated this revision to Diff 16925.May 26 2016, 5:28 PM
  • Use proper free param
bdrewery updated this revision to Diff 16926.May 26 2016, 5:30 PM
  • Grab and coffee and really use the right syntax
kib added inline comments.May 26 2016, 9:01 PM
sys/kern/kern_exec.c
645

I think you need imgp->execpath = NULL;

bdrewery added inline comments.May 26 2016, 9:03 PM
sys/kern/kern_exec.c
645

Sure, that would be safest. I think it would always be overwritten above though. I'll add it though.

bdrewery updated this revision to Diff 16939.May 26 2016, 9:07 PM
  • NULL imgp->execpath when handling interpreter
kib added inline comments.May 26 2016, 9:08 PM
sys/kern/kern_exec.c
645

At least, it would allow potential imgp consumers to not care about stage of the do_execve() where they act. Either pointer is null, or it points to current path.

kib accepted this revision.May 26 2016, 9:09 PM
kib edited edge metadata.
This revision is now accepted and ready to land.May 26 2016, 9:09 PM
Closed by commit rS300793: exec: Provide execpath in imgp for the process_exec hook. (authored by bdrewery). · Explain WhyMay 26 2016, 11:19 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
kern/
23 lines

Diff 16924

View Options

sys/kern/kern_exec.c

Loading...