Remove a needlessly clever hack to start init with sys_exec().

[brooks]

Summary

Construct a struct image_args with the help of new exec_args_*() helper
functions and call kern_execve().

The previous code mapped a page in userspace, copied arguments out
to it one at a time, and then constructed a struct execve_args all so
that sys_execve() can call exec_copyin_args() to copy the data back in
to a struct image_args.

Opencode the part of pre_execve()/post_execve() that releases a reference
to the initial vmspace. We don't need to stop threads like they do.

Obtained from: CheriBSD
Sponsored by: DARPA, AFRL
Depends on: D15468

[brooks]

• Rebase
• Catch up with changes in rS341263.

[jhb]

sys/kern/init_main.c

781–782

The reordering makes the diff a bit harder to read. Did you sort them alphabetically it seems?

783–788

This handling now seems kind of wrong. Previously this was adding the '-' in front of all of the options if there were any, now it is adding a second '-' so that you get '--' if there were no options. You could instead perhaps keep the 'options = 1' from the previous code and instead and perhaps just omit flags entirely if options is 0, e.g.:

     options = 0;
     flagp = &flags[0];
     *flagp++ = '-';
#ifdef BOOTCDROM
     *flagp++ = 'C'
     options = 1;
#endif
      /* other options */
      *flagp++ = 0;
      KASSERT(...);
      if (options != 0) {
          error = exec_args_add_arg(&args, flags, UIO_SYSSPACE);
          if (error != 0)
              panic(...);
      }

This seems cleaner than adding a dummy '--' option.

[kib]

Not calling pre/post leave a stray reference on vmspace0. I do not claim that this is important, but it is certainly something to consider and at least comment about. Might be the stray TDP_EXECVMSPC on thread0 is worse.

[brooks]

sys/kern/init_main.c

781–782

The they are reversed because the old code builds the argument backwards (from the top of the pseudo stack).

783–788

That doesn't match my read of the old code. The old options==0 case adds the a second '-' if there weren't any options and the code at line 784 adds the first hyphen unconditionally.

[brooks]

In D15469#391417, @kib wrote:

Not calling pre/post leave a stray reference on vmspace0. I do not claim that this is important, but it is certainly something to consider and at least comment about. Might be the stray TDP_EXECVMSPC on thread0 is worse.

Hmm, I'll look into this more closely on Monday. It's not particularly hard to call them.

[jhb]

sys/kern/init_main.c

781–782

Ok. I don't think the order of the flags passed to init matters, so would probably have preserved the existing code order to minimize the diff, but either way is fine.

783–788

Huh, that seems kind of silly to pass --. I guess it was because the old code always copied out arg1 and didn't want to deal with having it be optional. Maybe as a followup we can remove the '--' as I do think it's just a kludge to avoid having arg1 being optional. The 'options = 1' bit would reduce the diff a little still.

[kib]

In D15469#391421, @brooks wrote:

In D15469#391417, @kib wrote:

Not calling pre/post leave a stray reference on vmspace0. I do not claim that this is important, but it is certainly something to consider and at least comment about. Might be the stray TDP_EXECVMSPC on thread0 is worse.

Hmm, I'll look into this more closely on Monday. It's not particularly hard to call them.

No, I think that calling them on proc0 would have too much undesirable effects, we do not want to stop almost all kernel threads. You would need to open-code clearing of the flag and possible dereference of vmspace0.

[brooks]

In D15469#391506, @kib wrote:

In D15469#391421, @brooks wrote:

In D15469#391417, @kib wrote:

Not calling pre/post leave a stray reference on vmspace0. I do not claim that this is important, but it is certainly something to consider and at least comment about. Might be the stray TDP_EXECVMSPC on thread0 is worse.

Hmm, I'll look into this more closely on Monday. It's not particularly hard to call them.

No, I think that calling them on proc0 would have too much undesirable effects, we do not want to stop almost all kernel threads. You would need to open-code clearing of the flag and possible dereference of vmspace0.

I glanced at the pre/post code briefly on Friday and was thinking of open coding these bit as well. FWIW, we've been calling pre/post since they were added to sys_execve...

[brooks]

• Rebase
• Reduce diffs to master by retaining option handling order.
• Free vmspace reference and clear TDP_EXECVMSPC after kern_execve().

[brooks]

sys/kern/init_main.c

783–788

After this change, I'm going to do a pass removing BOOTCDROM (init warns and ignores about -C), the RB_FASTBOOT case (ifdef notyet since at least the 4.4lite import), and --. That will leave a single case of exec_args_add_arg(&args, "-s", UIS_SYSSPACE).

[kib]

sys/kern/init_main.c

797–809

error cannot be non-zero. You panic a line before.

[brooks]

• Remove redundent check (too much copy-and-paste).
• Fix typo.