Re: [PATCH]Modify do_exec() handler to deal with multiple imgact handlers

On Wed, 2014-09-03 at 15:39 -0400, John Baldwin wrote:
> On Tuesday, September 02, 2014 03:59:17 PM Sean Bruno wrote:
> > https://reviews.freebsd.org/D696
> > 
> > I found that the binmisc handler was not executing if the shell handler
> > fired.  Both were using the same intepreted flag to determine if they
> > should run.
> > 
> > This change modifies struct image_params.interpreted to be a bitfield
> > instead of a bool flag and assigns one bit to each image activator.
> > 
> > Comments?
> > 
> > sean
> > 
> > Index: sys/kern/imgact_binmisc.c
> > ===================================================================
> > --- sys/kern/imgact_binmisc.c
> > +++ sys/kern/imgact_binmisc.c
> > _at__at_ -600,12 +600,12 _at__at_
> >  	}
> > 
> >  	/* No interpreter nesting allowed. */
> > -	if (imgp->interpreted) {
> > +	if (imgp->interpreted & IMGACT_BINMISC) {
> >  		mtx_unlock(&interp_list_mtx);
> >  		return (ENOEXEC);
> >  	}
> > 
> > -	imgp->interpreted = 1;
> > +	imgp->interpreted |= IMGACT_BINMISC;
> > 
> >  	if (imgp->args->fname != NULL) {
> >  		fname = imgp->args->fname;
> > Index: sys/kern/imgact_shell.c
> > ===================================================================
> > --- sys/kern/imgact_shell.c
> > +++ sys/kern/imgact_shell.c
> > _at__at_ -115,10 +115,10 _at__at_
> >  	 * Don't allow a shell script to be the shell for a shell
> >  	 *	script. :-)
> >  	 */
> > -	if (imgp->interpreted)
> > +	if (imgp->interpreted & IMGACT_SHELL)
> >  		return (ENOEXEC);
> > 
> > -	imgp->interpreted = 1;
> > +	imgp->interpreted |= IMGACT_SHELL;
> > 
> >  	/*
> >  	 * At this point we have the first page of the file mapped.
> > Index: sys/sys/imgact.h
> > ===================================================================
> > --- sys/sys/imgact.h
> > +++ sys/sys/imgact.h
> > _at__at_ -61,7 +61,9 _at__at_
> >  	unsigned long entry_addr; /* entry address of target executable */
> >  	unsigned long reloc_base; /* load address of image */
> >  	char vmspace_destroyed;	/* flag - we've blown away original vm space */
> > -	char interpreted;	/* flag - this executable is interpreted */
> > +#define IMGACT_SHELL	0x1
> > +#define IMGACT_BINMISC	0x2
> > +	unsigned char interpreted;	/* mask of interpretes that have run */
> 
> s/interpretes/interpreters/
> 

Fixed on phabric review.

> Other than that I think this is fine, though I wonder if it will result
> in some unexpected effects (you probably want to be able to use a binmisc
> binary as the #! interpreter for a script, but I'm not sure the opposite
> is true.
> 

Its slightly more complicated by the fact that qemu-user has its own #!
parsing too.  If qemu-user sees that it is operating on a shell script,
it will make the needed changes itself to argv[0].  So the condition you
describe, with respect to qemu-user + binmisc won't happen.

ref qemu-bsd-user tree,
bsd-user/freebsd/os-proc.c:is_target_shell_script() and
freebsd_exec_common()

https://github.com/seanbruno/qemu-bsd-user/blob/bsd-user/bsd-user/freebsd/os-proc.c