Re: more fexecve questions
Please add a comment explaining that this is for pathnames we can't
resolve in chroots.
> @@ -224,9 +225,11 @@ elf_populate_auxv(struct lwp *l, struct
> a->a_v = l->l_proc->p_stackbase;
> a++;
>
> - execname = a;
> - a->a_type = AT_SUN_EXECNAME;
> - a++;
> + if (path[0] == '/' && path[1] != '\0') {
> + execname = a;
> + a->a_type = AT_SUN_EXECNAME;
> + a++;
> + }
>
> exec_free_emul_arg(pack);
> } else {
segvguard and veriexec ony use the name for printing error messages.
I don't think it's necessary to give up on them because of it.
segvguard even handles a NULL name already.
> + if (epp->ep_resolvedname) {
> #if NVERIEXEC > 0
> - error = veriexec_verify(l, vp, epp->ep_resolvedname,
> - epp->ep_flags & EXEC_INDIR ? VERIEXEC_INDIRECT : VERIEXEC_DIRECT,
> - NULL);
> - if (error)
> - goto bad2;
> + error = veriexec_verify(l, vp, epp->ep_resolvedname,
> + epp->ep_flags & EXEC_INDIR ? VERIEXEC_INDIRECT
> + : VERIEXEC_DIRECT, NULL);
> + if (error)
> + goto bad2;
> #endif /* NVERIEXEC > 0 */
>
> #ifdef PAX_SEGVGUARD
> - error = pax_segvguard(l, vp, epp->ep_resolvedname, false);
> - if (error)
> - goto bad2;
> + error = pax_segvguard(l, vp, epp->ep_resolvedname, false);
> + if (error)
> + goto bad2;
> #endif /* PAX_SEGVGUARD */
> + }
>
We really should stop stubbing out stuff like this. I wouldn't be
surprised if working around things would be easier if we didn't have the
function they're (rightly) feature-testing for.
(I had similar issues with fallocate...)
> @@ -559,7 +598,8 @@ sys_fexecve(struct lwp *l, const struct
> syscallarg(char * const *) envp;
> } */
>
> - return ENOSYS;
> + return execve1(l, NULL, SCARG(uap, fd), SCARG(uap, argp),
> + SCARG(uap, envp), execve_fetch_element);
> }
>
> /*
Re: CVS commit: src/sys/kern
Michael van Elst wrote:
> A real solution would just support the NAME=* syntax in getwedgename().
> It might also allow for a case-insensitive match like userland. Then
> it would just work for config, for boot parameters and for interactive
> entries.
You mean this change?
--- sys/kern/kern_subr.c27 Jan 2019 02:08:43 - 1.223
+++ sys/kern/kern_subr.c15 Sep 2019 01:46:42 -
@@ -678,15 +678,20 @@
static const char *
getwedgename(const char *name, int namelen)
{
- const char *wpfx = "wedge:";
- const int wpfxlen = strlen(wpfx);
+ const char *wpfx1 = "wedge:";
+ const char *wpfx2 = "NAME=";
+ const int wpfx1len = strlen(wpfx1);
+ const int wpfx2len = strlen(wpfx2);
- if (namelen < wpfxlen || strncmp(name, wpfx, wpfxlen) != 0)
- return NULL;
+ if (namelen > wpfx1len && strncmp(name, wpfx1, wpfx1len) == 0)
+ return name + wpfx1len;
- return name + wpfxlen;
+ if (namelen > wpfx2len && strncmp(name, wpfx2, wpfx2len) == 0)
+ return name + wpfx2len;
+
+ return NULL;
}
static device_t
parsedisk(char *str, int len, int defpart, dev_t *devp)
--
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
m...@netbsd.org
Re: more fexecve questions
In article <20190910145247.c52cc17f...@rebar.astron.com>,
Christos Zoulas wrote:
1. So the consensus is to leave the file descriptor alone.
2. I've added the reverse cache lookup, and now script execution works.
3. Nobody suggested anything to improve security.
Here's the latest patch; if I don't hear objections soon I will commit it...
Index: compat/netbsd32/netbsd32_execve.c
===
RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_execve.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 netbsd32_execve.c
--- compat/netbsd32/netbsd32_execve.c 3 Sep 2018 16:29:29 - 1.39
+++ compat/netbsd32/netbsd32_execve.c 15 Sep 2019 00:14:10 -
@@ -71,9 +71,8 @@ netbsd32_execve(struct lwp *l, const str
syscallarg(netbsd32_charpp) argp;
syscallarg(netbsd32_charpp) envp;
} */
- const char *path = SCARG_P32(uap, path);
- return execve1(l, path, SCARG_P32(uap, argp),
+ return execve1(l, SCARG_P32(uap, path), -1, SCARG_P32(uap, argp),
SCARG_P32(uap, envp), netbsd32_execve_fetch_element);
}
@@ -86,13 +85,9 @@ netbsd32_fexecve(struct lwp *l, const st
syscallarg(netbsd32_charpp) argp;
syscallarg(netbsd32_charpp) envp;
} */
- struct sys_fexecve_args ua;
- NETBSD32TO64_UAP(fd);
- NETBSD32TOP_UAP(argp, char * const);
- NETBSD32TOP_UAP(envp, char * const);
-
- return sys_fexecve(l, , retval);
+ return execve1(l, NULL, SCARG(uap, fd), SCARG_P32(uap, argp),
+ SCARG_P32(uap, envp), netbsd32_execve_fetch_element);
}
static int
Index: kern/exec_elf.c
===
RCS file: /cvsroot/src/sys/kern/exec_elf.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 exec_elf.c
--- kern/exec_elf.c 7 Jun 2019 23:35:52 - 1.98
+++ kern/exec_elf.c 15 Sep 2019 00:14:10 -
@@ -157,6 +157,7 @@ elf_populate_auxv(struct lwp *l, struct
size_t len, vlen;
AuxInfo ai[ELF_AUX_ENTRIES], *a, *execname;
struct elf_args *ap;
+ char *path = l->l_proc->p_path;
int error;
a = ai;
@@ -224,9 +225,11 @@ elf_populate_auxv(struct lwp *l, struct
a->a_v = l->l_proc->p_stackbase;
a++;
- execname = a;
- a->a_type = AT_SUN_EXECNAME;
- a++;
+ if (path[0] == '/' && path[1] != '\0') {
+ execname = a;
+ a->a_type = AT_SUN_EXECNAME;
+ a++;
+ }
exec_free_emul_arg(pack);
} else {
@@ -242,7 +245,6 @@ elf_populate_auxv(struct lwp *l, struct
KASSERT(vlen <= sizeof(ai));
if (execname) {
- char *path = l->l_proc->p_path;
execname->a_v = (uintptr_t)(*stackp + vlen);
len = strlen(path) + 1;
if ((error = copyout(path, (*stackp + vlen), len)) != 0)
Index: kern/exec_script.c
===
RCS file: /cvsroot/src/sys/kern/exec_script.c,v
retrieving revision 1.79
diff -u -p -u -r1.79 exec_script.c
--- kern/exec_script.c 27 Jan 2019 02:08:43 - 1.79
+++ kern/exec_script.c 15 Sep 2019 00:14:10 -
@@ -290,7 +290,7 @@ check_shell:
/* try loading the interpreter */
if ((error = exec_makepathbuf(l, shellname, UIO_SYSSPACE,
_pathbuf, NULL)) == 0) {
- error = check_exec(l, epp, shell_pathbuf);
+ error = check_exec(l, epp, shell_pathbuf, NULL);
pathbuf_destroy(shell_pathbuf);
}
Index: kern/kern_exec.c
===
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.479
diff -u -p -u -r1.479 kern_exec.c
--- kern/kern_exec.c7 Sep 2019 15:34:44 - 1.479
+++ kern/kern_exec.c15 Sep 2019 00:14:10 -
@@ -257,7 +257,7 @@ struct execve_data {
struct ps_strings ed_arginfo;
char*ed_argp;
const char *ed_pathstring;
- char*ed_resolvedpathbuf;
+ char*ed_resolvedname;
size_t ed_ps_strings_sz;
int ed_szsigcode;
size_t ed_argslen;
@@ -309,9 +309,32 @@ exec_path_free(struct execve_data *data)
{
pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring);
pathbuf_destroy(data->ed_pathbuf);
- PNBUF_PUT(data->ed_resolvedpathbuf);
+ if (data->ed_resolvedname)
+ PNBUF_PUT(data->ed_resolvedname);
}
+static void
+exec_resolvename(struct lwp *l, struct exec_package *epp, struct vnode *vp,
+char **rpath)
+{
+ int error;
+ char *p;
+
+ KASSERT(rpath != NULL);
+
+ *rpath =
Re: fcntl(F_GETPATH) support
In article <2f29ca9a-0ae1-48d3-b3f4-1556912d4...@me.com>, Jason Thorpe wrote: > > >> On Sep 14, 2019, at 2:52 PM, Kamil Rytarowski wrote: >> >> On 14.09.2019 23:34, Christos Zoulas wrote: >>> Comments? >>> >> >> Question. How does it handle symbolic/hardlinks? > >Symbolic links, of course, are simply continuations of the lookup, so >there's "nothing to see here" with a symbolic link. If you can get a file desriptor to a symlink, it will work; I don't think that we have a way to do this now. >I looked at cache_revlookup() and it looks to me like it simply picks >the first hit it finds in the hash table for the given vnode. That is correct. >At least one platform that support this API uses "the last name the file >was looked up with", and will fall back on "whatever the underlying file >system considers to be the canonical name for the file", which is file >system-defined, if for some reason there is no entry in the name cache >(which could, for example, happen if an application is doing an >"open-by-file-id" type operation and the file has not yet been opened by >path name). It will also fail if the entry has been evicted from the cache, but that rarely happens with recently opened files. christos
Re: fcntl(F_GETPATH) support
> On Sep 14, 2019, at 2:52 PM, Kamil Rytarowski wrote: > > On 14.09.2019 23:34, Christos Zoulas wrote: >> Comments? >> > > Question. How does it handle symbolic/hardlinks? Symbolic links, of course, are simply continuations of the lookup, so there's "nothing to see here" with a symbolic link. I looked at cache_revlookup() and it looks to me like it simply picks the first hit it finds in the hash table for the given vnode. At least one platform that support this API uses "the last name the file was looked up with", and will fall back on "whatever the underlying file system considers to be the canonical name for the file", which is file system-defined, if for some reason there is no entry in the name cache (which could, for example, happen if an application is doing an "open-by-file-id" type operation and the file has not yet been opened by path name). -- thorpej
Re: fcntl(F_GETPATH) support
On 14.09.2019 23:34, Christos Zoulas wrote:
> Comments?
>
Question. How does it handle symbolic/hardlinks?
> christos
>
>
> Index: kern/sys_descrip.c
> ===
> RCS file: /cvsroot/src/sys/kern/sys_descrip.c,v
> retrieving revision 1.34
> diff -u -p -u -r1.34 sys_descrip.c
> --- kern/sys_descrip.c26 Aug 2019 10:19:08 - 1.34
> +++ kern/sys_descrip.c14 Sep 2019 21:33:29 -
> @@ -315,6 +312,28 @@ do_fcntl_lock(int fd, int cmd, struct fl
> return error;
> }
>
> +static int
> +do_fcntl_getpath(struct lwp *l, file_t *fp, char *upath)
> +{
> + char *kpath;
> + int error;
> +
> + if (fp->f_type != DTYPE_VNODE)
> + return EOPNOTSUPP;
> +
> + kpath = PNBUF_GET();
> +
> + error = vnode_to_path(kpath, MAXPATHLEN, fp->f_vnode, l, l->l_proc);
> + if (error)
> + goto out;
> +
> + error = copyoutstr(kpath, upath, MAXPATHLEN, NULL);
> +out:
> + PNBUF_PUT(kpath);
> +
> + return error;
> +}
> +
> /*
> * The file control system call.
> */
> @@ -463,6 +482,10 @@ sys_fcntl(struct lwp *l, const struct sy
> error = (*fp->f_ops->fo_ioctl)(fp, FIOSETOWN, );
> break;
>
> + case F_GETPATH:
> + error = do_fcntl_getpath(l, fp, SCARG(uap, arg));
> + break;
> +
> default:
> error = EINVAL;
> }
> Index: sys/fcntl.h
> ===
> RCS file: /cvsroot/src/sys/sys/fcntl.h,v
> retrieving revision 1.50
> diff -u -p -u -r1.50 fcntl.h
> --- sys/fcntl.h 20 Feb 2018 18:20:05 - 1.50
> +++ sys/fcntl.h 14 Sep 2019 21:33:29 -
> @@ -193,6 +195,7 @@
> #define F_DUPFD_CLOEXEC 12 /* close on exec duplicated fd
> */
> #define F_GETNOSIGPIPE 13 /* get SIGPIPE disposition */
> #define F_SETNOSIGPIPE 14 /* set SIGPIPE disposition */
> +#define F_GETPATH 15 /* get pathname assosiated with
> file */
> #endif
>
> /* file descriptor flags (F_GETFD, F_SETFD) */
>
signature.asc
Description: OpenPGP digital signature
fcntl(F_GETPATH) support
Comments?
christos
Index: kern/sys_descrip.c
===
RCS file: /cvsroot/src/sys/kern/sys_descrip.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 sys_descrip.c
--- kern/sys_descrip.c 26 Aug 2019 10:19:08 - 1.34
+++ kern/sys_descrip.c 14 Sep 2019 21:33:29 -
@@ -315,6 +312,28 @@ do_fcntl_lock(int fd, int cmd, struct fl
return error;
}
+static int
+do_fcntl_getpath(struct lwp *l, file_t *fp, char *upath)
+{
+ char *kpath;
+ int error;
+
+ if (fp->f_type != DTYPE_VNODE)
+ return EOPNOTSUPP;
+
+ kpath = PNBUF_GET();
+
+ error = vnode_to_path(kpath, MAXPATHLEN, fp->f_vnode, l, l->l_proc);
+ if (error)
+ goto out;
+
+ error = copyoutstr(kpath, upath, MAXPATHLEN, NULL);
+out:
+ PNBUF_PUT(kpath);
+
+ return error;
+}
+
/*
* The file control system call.
*/
@@ -463,6 +482,10 @@ sys_fcntl(struct lwp *l, const struct sy
error = (*fp->f_ops->fo_ioctl)(fp, FIOSETOWN, );
break;
+ case F_GETPATH:
+ error = do_fcntl_getpath(l, fp, SCARG(uap, arg));
+ break;
+
default:
error = EINVAL;
}
Index: sys/fcntl.h
===
RCS file: /cvsroot/src/sys/sys/fcntl.h,v
retrieving revision 1.50
diff -u -p -u -r1.50 fcntl.h
--- sys/fcntl.h 20 Feb 2018 18:20:05 - 1.50
+++ sys/fcntl.h 14 Sep 2019 21:33:29 -
@@ -193,6 +195,7 @@
#defineF_DUPFD_CLOEXEC 12 /* close on exec duplicated fd
*/
#defineF_GETNOSIGPIPE 13 /* get SIGPIPE disposition */
#defineF_SETNOSIGPIPE 14 /* set SIGPIPE disposition */
+#defineF_GETPATH 15 /* get pathname assosiated with
file */
#endif
/* file descriptor flags (F_GETFD, F_SETFD) */
Re: NCHNAMLEN vnode cache limitation removal
> On Sep 14, 2019, at 1:20 AM, David Holland wrote: > >> Given the list of smart people I've seen discussing this, I'm >> presumably just missing something, but what? > > Loonix. Linux isn't the only system that has such a capability. -- thorpej
Re: NCHNAMLEN vnode cache limitation removal
On Fri, Sep 13, 2019 at 07:49:54PM -0400, Mouse wrote: > > [...], because fexecve is causing rumbles about doing significantly > > more reverse lookups. > > Why is everyone so concerned about finding "the" name of an inode, or > indeed any name for an inode? As far as I can tell there has never > been any promise that any given inode has any names pointing to it, at > least not unless it's got no other references to it (in which case it > would be freed if it had no names). > > Given the list of smart people I've seen discussing this, I'm > presumably just missing something, but what? Loonix. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/kern
Martin Husemann wrote: > Oh, *B*ootdev vs. *R*ootdev - I see. > Not sure why bootdev is important, however we should use the same syntax! The change handles the parameter passed from bootloader when booting Xen. In boot.cfg, NAME=label is now supported everywhere, and this was the only missing bit. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src/sys/kern
mar...@duskware.de (Martin Husemann) writes: >> config netbsd root on "wedge:Guru-Root" type ffs >> for ages (in the netbsd-7 branch even). >Oh, *B*ootdev vs. *R*ootdev - I see. >Not sure why bootdev is important, however we should use the same syntax! bootdev already acts as a default for rootdev. There is no difference. There should be no difference. The wedge:* syntax worked for ages. This change either should allow wedge names (ignorant of the wedge:* syntax) or align the naming with the NAME=* syntax. But like many other ad-hoc changes it is done wrong. A real solution would just support the NAME=* syntax in getwedgename(). It might also allow for a case-insensitive match like userland. Then it would just work for config, for boot parameters and for interactive entries. You may also want to think of augmenting dkwedge_print_wnames() to list the wedges in NAME= syntax and the documentation that currently only refers to wedge:*, in particular for config files. Or we may just accept that boot/config syntax is different from userland. Greetings, -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/kern
On Sat, Sep 14, 2019 at 06:26:34AM +, Martin Husemann wrote: > > Log Message: > > Accept root device specification as NAME=label [..] > Why is this needed? > > I have been using > > config netbsd root on "wedge:Guru-Root" type ffs > > for ages (in the netbsd-7 branch even). Oh, *B*ootdev vs. *R*ootdev - I see. Not sure why bootdev is important, however we should use the same syntax! Martin
Re: CVS commit: src/sys/kern
On Fri, Sep 13, 2019 at 01:33:20AM +, Emmanuel Dreyfus wrote: > Module Name: src > Committed By: manu > Date: Fri Sep 13 01:33:20 UTC 2019 > > Modified Files: > src/sys/kern: kern_subr.c > > Log Message: > Accept root device specification as NAME=label > > > To generate a diff of this commit: > cvs rdiff -u -r1.224 -r1.225 src/sys/kern/kern_subr.c Why is this needed? I have been using config netbsd root on "wedge:Guru-Root" type ffs for ages (in the netbsd-7 branch even). Martin
