exec: provide right hardlink name in AT_EXECPATH

[kib]

Summary

For this, use vn_fullpath_hardlink() to resolve executable name for
execve(2).

This should provide the right hardlink name, used for execution, instead
of random hardlink pointing to this binary.  Also this should make the
AT_EXECNAME reliable for execve(2), since kernel only needs to resolve
parent directory path, which should always succeed (except pathological
cases like unlinking a directory).

PR:     248184

exec: store parent directory and hardlink name of the binary in struct proc

sysctl kern.proc.procname: report right hardlink name

[mjg]

Can this only add WANTPARENT if vn_fullpath_hardlink is going to be used to begin with? The extra ref/unref on containing directory would preferably be avoided if it can be helped.

[kib]

In D32611#736134, @mjg wrote:

Can this only add WANTPARENT if vn_fullpath_hardlink is going to be used to begin with? The extra ref/unref on containing directory would preferably be avoided if it can be helped.

Isn't several jmps on modern CPUs cost same as atomic? I do not want to go with saving WANTPARENT in some var, it would be too ugly and really not save any jumps.

Anyway, below is update as you asked for. I also converted imgparam to bool as well. Per-commit view is available at https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/execpath

[kib]

Avoid WANTPARENT if not needed, i.e. fexecve(2) or path is absolute.
Convert imgparam to use bools, and group them together.

[kib]

Restore unconditional WANTPARENT: the binary' dvp is now saved in the struct proc to allow for kern.proc.pathname to return right name as well.

[markj]

sys/kern/kern_exec.c

439

Initializing oldtextdvp twice. I think it is technically safe to leave newtextvp uninitialized.

694

Don't we unconditionally set nd.ni_dvp = NULL above? i.e. the dvp reference is unconditionally transferred to newtextdvp after the namei() call.

[kib]

sys/kern/kern_exec.c

439

Perhaps yes, but following the mix of success/failure paths on the function exit is risky.

694

So yes this is true there, I changed the vrele to assert that ni_dvp == NULL.

I want the vrele(ni_dvp) to be left as is on exec_fail_dealloc: label. I think that technically ni_dvp cannot be non-null there, but it is too fragile to assume on the recovery path.

[kib]

Fix typo in NULL initialization.
Assert that ni_dvp is NULL, instead of releasing it, in the switch to interpreter' binary.

[markj]

sys/kern/kern_exec.c

439

I prefer the explicit initialization as well. I just wasn't sure if it was intentional.

694

Actually I do not understand how this works. Doesn't the use of vn_fullpath_hardlink() assume that ni_dvp != NULL?

sys/kern/kern_exit.c

437 ↗

(On Diff #97401)

Consider setting p->p_binname = NULL here, like we're doing for other fields.

[kib]

sys/kern/kern_exec.c

694

Right, there were too many moves around.

So I did one more, lets calculate the execpath right after namei(). I like the move on its own, because it naturally fit into fexecve(2) path of calculating execpath, also removing one vnode relock. Then ni_dvp is not used anywhere except this args->fname != NULL path.

Also I think it is possible to eliminate second NDINIT() by moving first one under the same if() branch.

[kib]

Coalesce most of args->fname != NULL processing, including interpreter NDINIT() reset of nd.
As consequence, audit interpreter path and remove XXX comment.
Use nd.ni_dvp locally and only care about reference on newtextdvp.
Set p_binname to NULL on exit.

[markj]

sys/kern/kern_exec.c

692

It took me a while to convince myself that we do not need to set imgp->vp = NULL here, but maybe it would be worthwhile anyway.

865

sys/kern/kern_proc.c

2280 ↗

(On Diff #97418)

There is some layering violation here I think, we are assuming that vn_fullpath_hardlink() only accesses these fields. I'm not sure how best to resolve it.

2291 ↗

(On Diff #97418)

If namei() fails we leak freebuf. It'll be overwritten by vn_fullpath().

[kib]

sys/kern/kern_exec.c

692

imgp is fully reset on interpreter recycle, I will add the reset.

[kib]

Change interface for vn_fullpath_hardlink: take vp/dvp/name instead of struct nameidata *
Leaks fixes
More reshuffling

[kib]

Fix vp/dvp juggling in vn_fullpath_hardlink()