d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve the MP safeness associated with the creation of symbolic

Browse Source 
links and the execution of ELF binaries. Two problems were found:

1) The link path wasn't tagged as being MP safe and thus was not properly
   protected.
2) The ELF interpreter vnode wasnt being locked in namei(9) and thus was
   insufficiently protected.

This commit makes the following changes:

-Sets the MPSAFE flag in NDINIT for symbolic link paths
-Sets the MPSAFE flag in NDINIT and introduce a vfslocked variable which
 will be used to instruct VFS_UNLOCK_GIANT to unlock Giant if it has been
 picked up.
-Drop in an assertion into vfs_lookup which ensures that if the MPSAFE
 flag is NOT set, that we have picked up giant. If not panic (if WITNESS
 compiled into the kernel). This should help us find conditions where vnode
 operations are in-sufficiently protected.

This is a RELENG_6 candidate.

Discussed with:	jeff
MFC after:	4 days
This commit is contained in:
Christian S.J. Peron 2005-09-15 15:03:48 +00:00
parent ac8189712e
commit 68ff2a4397
4 changed files with 11 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
sys/kern/imgact_elf.c
View File

@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/malloc.h>
#include <sys/mount.h>
#include <sys/mutex.h>
#include <sys/mman.h>
#include <sys/namei.h>
@ -515,7 +516,7 @@ __elfN(load_file)(struct proc *p, const char *file, u_long *addr,
vm_prot_t prot;
u_long rbase;
u_long base_addr = 0;
int error, i, numsegs;
int vfslocked, error, i, numsegs;
if (curthread->td_proc != p)
panic("elf_load_file - thread"); /* XXXKSE DIAGNOSTIC */
@ -536,12 +537,14 @@ __elfN(load_file)(struct proc *p, const char *file, u_long *addr,
imgp->execlabel = NULL;
/* XXXKSE */
NDINIT(nd, LOOKUP, LOCKLEAF|FOLLOW, UIO_SYSSPACE, file, curthread);
NDINIT(nd, LOOKUP, MPSAFE|LOCKLEAF|FOLLOW, UIO_SYSSPACE, file,
curthread);
vfslocked = 0;
if ((error = namei(nd)) != 0) {
nd->ni_vp = NULL;
goto fail;
}
vfslocked = NDHASGIANT(nd);
NDFREE(nd, NDF_ONLY_PNBUF);
imgp->vp = nd->ni_vp;
@ -629,6 +632,7 @@ __elfN(load_file)(struct proc *p, const char *file, u_long *addr,
if (nd->ni_vp)
vrele(nd->ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
free(tempdata, M_TEMP);
return (error);

2
sys/kern/vfs_extattr.c
View File

@ -1423,7 +1423,7 @@ kern_link(struct thread *td, char *path, char *link, enum uio_seg segflg)
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}
NDINIT(&nd, CREATE, LOCKPARENT | SAVENAME, segflg, link, td);
NDINIT(&nd, CREATE, LOCKPARENT | SAVENAME | MPSAFE, segflg, link, td);
if ((error = namei(&nd)) == 0) {
lvfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {

2
sys/kern/vfs_lookup.c
View File

@ -120,6 +120,8 @@ namei(ndp)
struct proc *p = td->td_proc;
int vfslocked;
KASSERT((cnp->cn_flags & MPSAFE) != 0 || mtx_owned(&Giant) != 0,
("NOT MPSAFE and Giant not held"));
ndp->ni_cnd.cn_cred = ndp->ni_cnd.cn_thread->td_ucred;
KASSERT(cnp->cn_cred && p, ("namei: bad cred/proc"));
KASSERT((cnp->cn_nameiop & (~OPMASK)) == 0,

2
sys/kern/vfs_syscalls.c
View File

@ -1423,7 +1423,7 @@ kern_link(struct thread *td, char *path, char *link, enum uio_seg segflg)
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}
NDINIT(&nd, CREATE, LOCKPARENT | SAVENAME, segflg, link, td);
NDINIT(&nd, CREATE, LOCKPARENT | SAVENAME | MPSAFE, segflg, link, td);
if ((error = namei(&nd)) == 0) {
lvfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {