On Sun, 2020-12-27 at 11:18 -0700, Theo de Raadt wrote:
> fork_filter_process() does not feel like the right name for
> the function anymore.
>
Why not? Right now we do fork and call system system in the child.
With my diff we move to fork -> exec
The fork part is most definitely still there. Or do you object to some
other part of the name?
> Take note the exit value of the process (as seen by wait elsewhere) will
> be subtly differe after this conversion from system() to execve().
> Upon failure, rather than being 127, it is now 1.
parent_sig_handler() for the SIGCHLD case does:
if (WEXITSTATUS(status) != 0) {
A little further down we also do:
mda_sysexit = WEXITSTATUS(status);
But mda_sysexit is only used for CHILD_MDA.
So I don't see a reason why this subtle difference matters here.
>
> Martijn van Duren <[email protected]> wrote:
>
> > Because filters use system(3) after forking we get 2 processes for every
> > filter: one for waiting for system(3) to return and one running the actual
> > filter.
> >
> > Since the extra smtpd process does absolutely nothing we can just as easily
> > copy over what system(3) does internally for execve and call the shell
> > command directly.
> >
> > OK?
> >
> > martijn@
> >
> > Index: smtpd.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
> > retrieving revision 1.335
> > diff -u -p -r1.335 smtpd.c
> > --- smtpd.c 23 Sep 2020 19:11:50 -0000 1.335
> > +++ smtpd.c 27 Dec 2020 17:39:02 -0000
> > @@ -1304,7 +1304,9 @@ fork_filter_process(const char *name, co
> > struct passwd *pw;
> > struct group *gr;
> > char exec[_POSIX_ARG_MAX];
> > + char *argp[] = { "sh", "-c", exec, NULL };
> > int execr;
> > + extern char **environ;
> >
> > if (user == NULL)
> > user = SMTPD_USER;
> > @@ -1387,11 +1389,8 @@ fork_filter_process(const char *name, co
> > */
> > if (read(STDERR_FILENO, &buf, 1) != 0)
> > errx(1, "lka didn't properly close write end of error
> > socket");
> > - if (system(exec) == -1)
> > - err(1, NULL);
> > -
> > - /* there's no successful exit from a processor */
> > - _exit(1);
> > + execve(_PATH_BSHELL, argp, environ);
> > + err(1, "execve");
> > }
> >
> > static void
> >
> >
