From owner-freebsd-current@FreeBSD.ORG Wed Sep 3 23:32:01 2014 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2DABB84C; Wed, 3 Sep 2014 23:32:01 +0000 (UTC) Received: from mail.ignoranthack.me (ignoranthack.me [199.102.79.106]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0D4931902; Wed, 3 Sep 2014 23:32:00 +0000 (UTC) Received: from [192.168.200.205] (unknown [50.136.155.142]) (using SSLv3 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sbruno@ignoranthack.me) by mail.ignoranthack.me (Postfix) with ESMTPSA id 0E8E0192906; Wed, 3 Sep 2014 23:31:52 +0000 (UTC) Subject: Re: [PATCH]Modify do_exec() handler to deal with multiple imgact handlers From: Sean Bruno Reply-To: sbruno@freebsd.org To: John Baldwin In-Reply-To: <1737767.X0j5Yf1ak8@ralph.baldwin.cx> References: <1409698757.55485.8.camel@bruno> <1737767.X0j5Yf1ak8@ralph.baldwin.cx> Content-Type: text/plain; charset="us-ascii" Date: Wed, 03 Sep 2014 16:31:50 -0700 Message-ID: <1409787110.1137.8.camel@bruno> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Sep 2014 23:32:01 -0000 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 > > @@ -600,12 +600,12 @@ > > } > > > > /* 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 > > @@ -115,10 +115,10 @@ > > * 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 > > @@ -61,7 +61,9 @@ > > 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