svn commit: r367456 - head/sys/kern
Kyle Evans
kevans at FreeBSD.org
Sat Nov 7 18:07:56 UTC 2020
Author: kevans
Date: Sat Nov 7 18:07:55 2020
New Revision: 367456
URL: https://svnweb.freebsd.org/changeset/base/367456
Log:
imgact_binmisc: move some calculations out of the exec path
The offset we need to account for in the interpreter string comes in two
variants:
1. Fixed - macros other than #a that will not vary from invocation to
invocation
2. Variable - #a, which is substitued with the argv0 that we're replacing
Note that we don't have a mechanism to modify an existing entry. By
recording both of these offset requirements when the interpreter is added,
we can avoid some unnecessary calculations in the exec path.
Most importantly, we can know up-front whether we need to grab
calculate/grab the the filename for this interpreter. We also get to avoid
walking the string a first time looking for macros. For most invocations,
it's a swift exit as they won't have any, but there's no point entering a
loop and searching for the macro indicator if we already know there will not
be one.
While we're here, go ahead and only calculate the argv0 name length once per
invocation. While it's unlikely that we'll have more than one #a, there's no
reason to recalculate it every time we encounter an #a when it will not
change.
I have not bothered trying to benchmark this at all, because it's arguably a
minor and straightforward/obvious improvement.
MFC after: 1 week
Modified:
head/sys/kern/imgact_binmisc.c
Modified: head/sys/kern/imgact_binmisc.c
==============================================================================
--- head/sys/kern/imgact_binmisc.c Sat Nov 7 17:18:44 2020 (r367455)
+++ head/sys/kern/imgact_binmisc.c Sat Nov 7 18:07:55 2020 (r367456)
@@ -63,8 +63,10 @@ typedef struct imgact_binmisc_entry {
uint8_t *ibe_magic;
uint8_t *ibe_mask;
uint8_t *ibe_interpreter;
+ ssize_t ibe_interp_offset;
uint32_t ibe_interp_argcnt;
uint32_t ibe_interp_length;
+ uint32_t ibe_argv0_cnt;
uint32_t ibe_flags;
uint32_t ibe_moffset;
uint32_t ibe_msize;
@@ -154,7 +156,8 @@ imgact_binmisc_populate_interp(char *str, imgact_binmi
* Allocate memory and populate a new entry for the interpreter table.
*/
static imgact_binmisc_entry_t *
-imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe)
+imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe, ssize_t interp_offset,
+ int argv0_cnt)
{
imgact_binmisc_entry_t *ibe = NULL;
size_t namesz = min(strlen(xbe->xbe_name) + 1, IBE_NAME_MAX);
@@ -175,7 +178,8 @@ imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe)
ibe->ibe_moffset = xbe->xbe_moffset;
ibe->ibe_msize = xbe->xbe_msize;
ibe->ibe_flags = xbe->xbe_flags;
-
+ ibe->ibe_interp_offset = interp_offset;
+ ibe->ibe_argv0_cnt = argv0_cnt;
return (ibe);
}
@@ -227,7 +231,8 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
{
imgact_binmisc_entry_t *ibe;
char *p;
- int cnt;
+ ssize_t interp_offset;
+ int argv0_cnt, cnt;
if (xbe->xbe_msize > IBE_MAGIC_MAX)
return (EINVAL);
@@ -242,23 +247,21 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
/* Make sure we don't have any invalid #'s. */
p = xbe->xbe_interpreter;
- while (1) {
- p = strchr(p, '#');
- if (!p)
- break;
-
+ interp_offset = 0;
+ argv0_cnt = 0;
+ while ((p = strchr(p, '#')) != NULL) {
p++;
switch(*p) {
case ISM_POUND:
/* "##" */
p++;
+ interp_offset--;
break;
-
case ISM_OLD_ARGV0:
/* "#a" */
p++;
+ argv0_cnt++;
break;
-
case 0:
default:
/* Anything besides the above is invalid. */
@@ -273,7 +276,7 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
}
/* Preallocate a new entry. */
- ibe = imgact_binmisc_new_entry(xbe);
+ ibe = imgact_binmisc_new_entry(xbe, interp_offset, argv0_cnt);
SLIST_INSERT_HEAD(&interpreter_list, ibe, link);
interp_list_entry_count++;
@@ -586,12 +589,16 @@ imgact_binmisc_exec(struct image_params *imgp)
const char *image_header = imgp->image_header;
const char *fname = NULL;
int error = 0;
- size_t offset, l;
+#ifdef INVARIANTS
+ int argv0_cnt = 0;
+#endif
+ size_t namelen, offset;
imgact_binmisc_entry_t *ibe;
struct sbuf *sname;
char *s, *d;
sname = NULL;
+ namelen = 0;
/* Do we have an interpreter for the given image header? */
INTERP_LIST_RLOCK();
if ((ibe = imgact_binmisc_find_interpreter(image_header)) == NULL) {
@@ -607,14 +614,22 @@ imgact_binmisc_exec(struct image_params *imgp)
imgp->interpreted |= IMGACT_BINMISC;
- if (imgp->args->fname != NULL) {
- fname = imgp->args->fname;
- } else {
- /* Use the fdescfs(5) path for fexecve(2). */
- sname = sbuf_new_auto();
- sbuf_printf(sname, "/dev/fd/%d", imgp->args->fd);
- sbuf_finish(sname);
- fname = sbuf_data(sname);
+ /*
+ * Don't bother with the overhead of putting fname together if we're not
+ * using #a.
+ */
+ if (ibe->ibe_argv0_cnt != 0) {
+ if (imgp->args->fname != NULL) {
+ fname = imgp->args->fname;
+ } else {
+ /* Use the fdescfs(5) path for fexecve(2). */
+ sname = sbuf_new_auto();
+ sbuf_printf(sname, "/dev/fd/%d", imgp->args->fd);
+ sbuf_finish(sname);
+ fname = sbuf_data(sname);
+ }
+
+ namelen = strlen(fname);
}
/*
@@ -622,39 +637,15 @@ imgact_binmisc_exec(struct image_params *imgp)
* we first shift all the other values in the `begin_argv' area to
* provide the exact amount of room for the values added. Set up
* `offset' as the number of bytes to be added to the `begin_argv'
- * area.
+ * area. ibe_interp_offset is the fixed offset from macros present in
+ * the interpreter string.
*/
- offset = ibe->ibe_interp_length;
+ offset = ibe->ibe_interp_length + ibe->ibe_interp_offset;
- /* Adjust the offset for #'s. */
- s = ibe->ibe_interpreter;
- while (1) {
- s = strchr(s, '#');
- if (!s)
- break;
+ /* Variable offset to be added from macros to the interpreter string. */
+ MPASS(ibe->ibe_argv0_cnt == 0 || namelen > 0);
+ offset += ibe->ibe_argv0_cnt * (namelen - 2);
- s++;
- switch(*s) {
- case ISM_POUND:
- /* "##" -> "#": reduce offset by one. */
- offset--;
- break;
-
- case ISM_OLD_ARGV0:
- /* "#a" -> (old argv0): increase offset to fit fname */
- offset += strlen(fname) - 2;
- break;
-
- default:
- /* Hmm... This shouldn't happen. */
- printf("%s: Unknown macro #%c sequence in "
- "interpreter string\n", KMOD_NAME, *(s + 1));
- error = EINVAL;
- goto done;
- }
- s++;
- }
-
/* Make room for the interpreter */
error = exec_args_adjust_args(imgp->args, 0, offset);
if (error != 0) {
@@ -680,26 +671,20 @@ imgact_binmisc_exec(struct image_params *imgp)
/* "##": Replace with a single '#' */
*d++ = '#';
break;
-
case ISM_OLD_ARGV0:
/* "#a": Replace with old arg0 (fname). */
- if ((l = strlen(fname)) != 0) {
- memcpy(d, fname, l);
- d += l;
- }
+ MPASS(ibe->ibe_argv0_cnt >= ++argv0_cnt);
+ memcpy(d, fname, namelen);
+ d += namelen;
break;
-
default:
- /* Shouldn't happen but skip it if it does. */
- break;
+ __assert_unreachable();
}
break;
-
case ' ':
/* Replace space with NUL to separate arguments. */
*d++ = '\0';
break;
-
default:
*d++ = *s;
break;
@@ -708,6 +693,8 @@ imgact_binmisc_exec(struct image_params *imgp)
}
*d = '\0';
+ /* Catch ibe->ibe_argv0_cnt counting more #a than we did. */
+ MPASS(ibe->ibe_argv0_cnt == argv0_cnt);
imgp->interpreter_name = imgp->args->begin_argv;
done:
More information about the svn-src-head
mailing list