From owner-freebsd-fs@FreeBSD.ORG Thu Jul 17 15:20:34 2008 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B4184106566B for ; Thu, 17 Jul 2008 15:20:34 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from mailhub.cs.uoguelph.ca (mailhub.cs.uoguelph.ca [131.104.94.205]) by mx1.freebsd.org (Postfix) with ESMTP id 789E28FC17 for ; Thu, 17 Jul 2008 15:20:34 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from muncher.cs.uoguelph.ca (muncher.cs.uoguelph.ca [131.104.91.102]) by mailhub.cs.uoguelph.ca (8.13.1/8.13.1) with ESMTP id m6HFKXhs002535; Thu, 17 Jul 2008 11:20:33 -0400 Received: from localhost (rmacklem@localhost) by muncher.cs.uoguelph.ca (8.11.7p3+Sun/8.11.6) with ESMTP id m6HFV3r25863; Thu, 17 Jul 2008 11:31:03 -0400 (EDT) X-Authentication-Warning: muncher.cs.uoguelph.ca: rmacklem owned process doing -bs Date: Thu, 17 Jul 2008 11:31:03 -0400 (EDT) From: Rick Macklem X-X-Sender: rmacklem@muncher.cs.uoguelph.ca To: Kostik Belousov In-Reply-To: <20080717110247.GI17123@deviant.kiev.zoral.com.ua> Message-ID: References: <20080715203641.GA17123@deviant.kiev.zoral.com.ua> <20080716154407.GG17123@deviant.kiev.zoral.com.ua> <20080717110247.GI17123@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Scanned-By: MIMEDefang 2.63 on 131.104.94.205 Cc: freebsd-fs@freebsd.org Subject: Re: executable open until unmount X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jul 2008 15:20:34 -0000 Retested modified patch and seems fine both ways. I also tried one that couldn't be opened. Thanks again, rick On Thu, 17 Jul 2008, Kostik Belousov wrote: > On Wed, Jul 16, 2008 at 06:44:07PM +0300, Kostik Belousov wrote: >> On Wed, Jul 16, 2008 at 11:32:28AM -0400, Rick Macklem wrote: >>> Patch looks good. It fixed my problem and hasn't crashed the system yet;-) >> Did you tested both elf executables and #!-scripts ? >> >>> >>> Thanks, rick > > And, in fact, the patch has a problem. Namely, it does not properly > track the opened status of the text vnode, because exec_check_permission() > could not opened it in case of error. > > Please, retest the change below. > > diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c > index f4335a2..e31ca37 100644 > --- a/sys/kern/kern_exec.c > +++ b/sys/kern/kern_exec.c > @@ -369,6 +369,7 @@ do_execve(td, args, mac_p) > imgp->entry_addr = 0; > imgp->vmspace_destroyed = 0; > imgp->interpreted = 0; > + imgp->opened = 0; > imgp->interpreter_name = args->buf + PATH_MAX + ARG_MAX; > imgp->auxargs = NULL; > imgp->vp = NULL; > @@ -496,6 +497,10 @@ interpret: > interplabel = mac_vnode_label_alloc(); > mac_vnode_copy_label(binvp->v_label, interplabel); > #endif > + if (imgp->opened) { > + VOP_CLOSE(binvp, FREAD, td->td_ucred, td); > + imgp->opened = 0; > + } > vput(binvp); > vm_object_deallocate(imgp->object); > imgp->object = NULL; > @@ -845,6 +850,8 @@ exec_fail_dealloc: > if (imgp->vp != NULL) { > if (args->fname) > NDFREE(ndp, NDF_ONLY_PNBUF); > + if (imgp->opened) > + VOP_CLOSE(imgp->vp, FREAD, td->td_ucred, td); > vput(imgp->vp); > } > > @@ -1326,6 +1333,8 @@ exec_check_permissions(imgp) > * general case). > */ > error = VOP_OPEN(vp, FREAD, td->td_ucred, td, NULL); > + if (error == 0) > + imgp->opened = 1; > return (error); > } > > diff --git a/sys/sys/imgact.h b/sys/sys/imgact.h > index 85eaea8..011a7ae 100644 > --- a/sys/sys/imgact.h > +++ b/sys/sys/imgact.h > @@ -58,6 +58,7 @@ struct image_params { > unsigned long entry_addr; /* entry address of target executable */ > char vmspace_destroyed; /* flag - we've blown away original vm space */ > char interpreted; /* flag - this executable is interpreted */ > + char opened; /* flag - we have opened executable vnode */ > char *interpreter_name; /* name of the interpreter */ > void *auxargs; /* ELF Auxinfo structure pointer */ > struct sf_buf *firstpage; /* first page that we mapped */ >