Large Capsicum patch for review.
Christoph Mallon
christoph.mallon at gmx.de
Sun Feb 24 15:07:22 UTC 2013
On 24.02.2013 07:05, Christoph Mallon wrote:
> I started reading the patch and found some minor glitches, e.g. mode in cap_sandboxed() should be u_int, not int.
> I will report more later.
I placed several patches with suggested changes at http://tron.homeunix.org/zeug/FreeBSD/capsicum/ (I also attached them).
Please have a look at them.
I also have some comments:
- A bitmask for cap_rights_limit() seems limited.
There are only six bits left unused.
Maybe a more general mechanism should be used.
- I think that CAP_SEND, CAP_RECV, CAP_FCHMODAT, CAP_FCHOWNAT, CAP_FSTATAT and CAP_FUTIMESAT are pointless aliases, which provide no benefit, but rather might cause confusion, whether there is a difference between e.g. CAP_WRITE and CAP_SEND.
- Why did you choose INT_MAX to represent "all capabilities"?
The data type used is ssize_t, so SSIZE_MAX seems more logical.
Further, there are several places where size_t and ssize_t come into contact, which need careful tests and casts to get right.
Maybe it is better to always use size_t and represent "all capabilities" with SIZE_T_MAX or (size_t)-1 (which is guaranteed by C to be the maximal value).
- Is it correct, that fdp seems to be not locked in fp_getfvp()?
Otherweise, fget_locked() could be used instead of the manual check.
- I could not verify many changes, e.g. changed requested permissions, because this is just a big diff with no information about the intention of the changes.
A series of smaller diffs with commit logs to state their intent would be really useful for reviewing.
Christoph
-------------- next part --------------
From 7f18186c451c235bcc3dbf2e25f65613e1ce7f16 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:01:00 +0100
Subject: [PATCH 02/24] Use the correct type for the parameter of
cap_getmode().
---
lib/libc/gen/cap_sandboxed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/libc/gen/cap_sandboxed.c b/lib/libc/gen/cap_sandboxed.c
index 95e5b23..5743f2a 100644
--- a/lib/libc/gen/cap_sandboxed.c
+++ b/lib/libc/gen/cap_sandboxed.c
@@ -39,7 +39,7 @@ __FBSDID("$FreeBSD$");
bool
cap_sandboxed(void)
{
- int mode;
+ u_int mode;
if (cap_getmode(&mode) == -1) {
assert(errno == ENOSYS);
--
1.8.1.3
-------------- next part --------------
From 7ad09a0cf0ceb9828171133e32f7148ffde62d97 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:03:03 +0100
Subject: [PATCH 03/24] Use cheaper != 0 tests.
---
lib/libc/gen/cap_sandboxed.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/libc/gen/cap_sandboxed.c b/lib/libc/gen/cap_sandboxed.c
index 5743f2a..f09b590 100644
--- a/lib/libc/gen/cap_sandboxed.c
+++ b/lib/libc/gen/cap_sandboxed.c
@@ -41,10 +41,10 @@ cap_sandboxed(void)
{
u_int mode;
- if (cap_getmode(&mode) == -1) {
+ if (cap_getmode(&mode) != 0) {
assert(errno == ENOSYS);
return (false);
}
assert(mode == 0 || mode == 1);
- return (mode == 1);
+ return (mode != 0);
}
--
1.8.1.3
-------------- next part --------------
From ae6ac8b82374e83dcc2bda7dd6e17da5d65d9eae Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:03:24 +0100
Subject: [PATCH 04/24] Add cap_sandboxed to Symbol.map.
---
lib/libc/sys/Symbol.map | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/libc/sys/Symbol.map b/lib/libc/sys/Symbol.map
index 376ace0..7738e46 100644
--- a/lib/libc/sys/Symbol.map
+++ b/lib/libc/sys/Symbol.map
@@ -384,6 +384,7 @@ FBSD_1.3 {
cap_ioctls_limit;
cap_rights_get;
cap_rights_limit;
+ cap_sandboxed;
clock_getcpuclockid2;
ffclock_getcounter;
ffclock_getestimate;
--
1.8.1.3
-------------- next part --------------
From 4822faec8ea5c53c673740eeec0cb604960af37b Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:03:59 +0100
Subject: [PATCH 05/24] Use standard inline instead of the GNUism __inline.
---
sys/kern/sys_capability.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index 941ea49..0f4ef12 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -144,7 +144,7 @@ sys_cap_getmode(struct thread *td, struct cap_getmode_args *uap)
FEATURE(security_capabilities, "Capsicum Capabilities");
-static __inline int
+static inline int
_cap_check(cap_rights_t have, cap_rights_t need, enum ktr_cap_fail_type type)
{
--
1.8.1.3
-------------- next part --------------
From 1d1defdf7dd117afa56330c958daeac5d52be05f Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:04:41 +0100
Subject: [PATCH 06/24] Avoid polluting the global namespace with stdbool.h.
---
sys/sys/capability.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sys/sys/capability.h b/sys/sys/capability.h
index df95ae4..9eed4d1 100644
--- a/sys/sys/capability.h
+++ b/sys/sys/capability.h
@@ -251,7 +251,6 @@ int cap_fcntl_check(struct filedesc *fdp, int fd, int cmd);
#else /* !_KERNEL */
__BEGIN_DECLS
-#include <stdbool.h>
/*
* cap_enter(): Cause the process to enter capability mode, which will
@@ -270,7 +269,7 @@ int cap_enter(void);
* Are we sandboxed (in capability mode)?
* This is libc wrapper around cap_getmode(2) system call.
*/
-bool cap_sandboxed(void);
+_Bool cap_sandboxed(void);
/*
* cap_getmode(): Are we in capability mode?
--
1.8.1.3
-------------- next part --------------
From 5107ab9aec6766c9ce5285939637c5739b2aed0c Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:05:19 +0100
Subject: [PATCH 07/24] Use ${CC} instead of hardcoding gcc.
---
tools/regression/capsicum/syscalls/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/regression/capsicum/syscalls/Makefile b/tools/regression/capsicum/syscalls/Makefile
index cb1b07b..5d34226 100644
--- a/tools/regression/capsicum/syscalls/Makefile
+++ b/tools/regression/capsicum/syscalls/Makefile
@@ -14,7 +14,7 @@ all: ${SYSCALLS} ${SYSCALLS:=.t}
.for SYSCALL in ${SYSCALLS}
${SYSCALL}: ${SYSCALL}.c misc.c
- gcc ${CFLAGS} ${@}.c misc.c -o $@
+ ${CC} ${CFLAGS} ${@}.c misc.c -o $@
${SYSCALL}.t: ${SYSCALL}
@printf "#!/bin/sh\n\n%s/%s\n" ${.CURDIR} ${@:.t=} > $@
--
1.8.1.3
-------------- next part --------------
From d2fe7a6f9d8e08943600b1f5d6a2818c6c4aa794 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:31:37 +0100
Subject: [PATCH 08/24] Use fget_locked() instead of duplicating it.
---
sys/kern/uipc_usrreq.c | 3 +--
sys/netsmb/smb_dev.c | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 15a1c8a..529d2eb 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1858,8 +1858,7 @@ unp_internalize(struct mbuf **controlp, struct thread *td)
FILEDESC_SLOCK(fdesc);
for (i = 0; i < oldfds; i++) {
fd = *fdp++;
- if (fd < 0 || fd >= fdesc->fd_nfiles ||
- fdesc->fd_ofiles[fd].fde_file == NULL) {
+ if (fget_locked(fdp, fd) == NULL) {
FILEDESC_SUNLOCK(fdesc);
error = EBADF;
goto out;
diff --git a/sys/netsmb/smb_dev.c b/sys/netsmb/smb_dev.c
index 0cee325..a09d74d 100644
--- a/sys/netsmb/smb_dev.c
+++ b/sys/netsmb/smb_dev.c
@@ -399,9 +399,7 @@ nsmb_getfp(struct filedesc* fdp, int fd, int flag)
struct file* fp;
FILEDESC_SLOCK(fdp);
- if (fd < 0 || fd >= fdp->fd_nfiles ||
- (fp = fdp->fd_ofiles[fd].fde_file) == NULL ||
- (fp->f_flag & flag) == 0) {
+ if ((fp = fget_locked(fdp, fd)) == NULL || (fp->f_flag & flag) == 0) {
FILEDESC_SUNLOCK(fdp);
return (NULL);
}
--
1.8.1.3
-------------- next part --------------
From cc2de42e944828943c17fcba2a821becc634b9a4 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:33:49 +0100
Subject: [PATCH 09/24] Remove duplicate checks.
fget_locked() just below does the same checks.
---
sys/kern/kern_descrip.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index bed5d8f..473ab40 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -2459,11 +2459,6 @@ fgetvp_rights(struct thread *td, int fd, cap_rights_t need,
if (td == NULL || (fdp = td->td_proc->p_fd) == NULL)
return (EBADF);
- FILEDESC_LOCK_ASSERT(fdp);
-
- if (fd < 0 || fd >= fdp->fd_nfiles)
- return (EBADF);
-
fp = fget_locked(fdp, fd);
if (fp == NULL || fp->f_ops == &badfileops)
return (EBADF);
--
1.8.1.3
-------------- next part --------------
From 2b9add7396a9485add6a4246071cf041a50161d1 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:34:43 +0100
Subject: [PATCH 10/24] Make fp_getfvp() static.
---
sys/fs/nfs/nfsdport.h | 2 --
sys/fs/nfsserver/nfs_nfsdport.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/sys/fs/nfs/nfsdport.h b/sys/fs/nfs/nfsdport.h
index 529ada2..a09a6dd 100644
--- a/sys/fs/nfs/nfsdport.h
+++ b/sys/fs/nfs/nfsdport.h
@@ -94,8 +94,6 @@ struct nfsexstuff {
#define NFSFPCRED(f) ((f)->f_cred)
#define NFSFPFLAG(f) ((f)->f_flag)
-int fp_getfvp(NFSPROC_T *, int, struct file **, struct vnode **);
-
#define NFSNAMEICNDSET(n, c, o, f) do { \
(n)->cn_cred = (c); \
(n)->cn_nameiop = (o); \
diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c
index cd7814c..880f965 100644
--- a/sys/fs/nfsserver/nfs_nfsdport.c
+++ b/sys/fs/nfsserver/nfs_nfsdport.c
@@ -2767,7 +2767,7 @@ out:
/*
* glue for fp.
*/
-int
+static int
fp_getfvp(struct thread *p, int fd, struct file **fpp, struct vnode **vpp)
{
struct filedesc *fdp;
--
1.8.1.3
-------------- next part --------------
From 4645cc6db54f29c1f5ec32c47794fd135f99cf60 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:37:22 +0100
Subject: [PATCH 11/24] Use simple assignment instead of bcopy() to copy
structs.
---
sys/kern/kern_descrip.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 473ab40..94bb74a 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -839,8 +839,7 @@ do_dup(struct thread *td, int flags, int old, int new,
/*
* Duplicate the source descriptor.
*/
- bcopy(&fdp->fd_ofiles[old], &fdp->fd_ofiles[new],
- sizeof(fdp->fd_ofiles[new]));
+ fdp->fd_ofiles[new] = fdp->fd_ofiles[old];
filecaps_copy(&fdp->fd_ofiles[old].fde_caps,
&fdp->fd_ofiles[new].fde_caps);
if ((flags & DUP_CLOEXEC) != 0) {
@@ -1374,7 +1373,7 @@ filecaps_copy(const struct filecaps *src, struct filecaps *dst)
{
size_t size;
- bcopy(src, dst, sizeof(*dst));
+ *dst = *src;
if (src->fc_ioctls != NULL) {
KASSERT(src->fc_nioctls > 0,
("fc_ioctls != NULL, but fc_nioctls=%hd", src->fc_nioctls));
@@ -1392,7 +1391,7 @@ static void
filecaps_move(struct filecaps *src, struct filecaps *dst)
{
- bcopy(src, dst, sizeof(*dst));
+ *dst = *src;
bzero(src, sizeof(*src));
}
@@ -1835,7 +1834,7 @@ fdcopy(struct filedesc *fdp)
(ofde->fde_file->f_ops->fo_flags & DFLAG_PASSABLE) &&
ofde->fde_file->f_ops != &badfileops) {
nfde = &newfdp->fd_ofiles[i];
- bcopy(ofde, nfde, sizeof(*nfde));
+ *nfde = *ofde;
filecaps_copy(&ofde->fde_caps, &nfde->fde_caps);
fhold(nfde->fde_file);
newfdp->fd_lastfile = i;
@@ -2681,8 +2680,7 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode,
return (EACCES);
}
fhold(fp);
- bcopy(&fdp->fd_ofiles[dfd], &fdp->fd_ofiles[indx],
- sizeof(fdp->fd_ofiles[indx]));
+ fdp->fd_ofiles[indx] = fdp->fd_ofiles[dfd];
filecaps_copy(&fdp->fd_ofiles[dfd].fde_caps,
&fdp->fd_ofiles[indx].fde_caps);
break;
@@ -2690,8 +2688,7 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode,
/*
* Steal away the file pointer from dfd and stuff it into indx.
*/
- bcopy(&fdp->fd_ofiles[dfd], &fdp->fd_ofiles[indx],
- sizeof(fdp->fd_ofiles[indx]));
+ fdp->fd_ofiles[indx] = fdp->fd_ofiles[dfd];
bzero(&fdp->fd_ofiles[dfd], sizeof(fdp->fd_ofiles[dfd]));
fdunused(fdp, dfd);
break;
--
1.8.1.3
-------------- next part --------------
From fc8878d0a43ea3f9d77b1fd51f90f4b4be188fb2 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:40:32 +0100
Subject: [PATCH 12/24] Remove redundant null pointer tests before free().
---
sys/kern/kern_descrip.c | 3 +--
sys/kern/sys_capability.c | 15 +++++----------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 94bb74a..53b24b1 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1415,8 +1415,7 @@ void
filecaps_free(struct filecaps *fcaps)
{
- if (fcaps->fc_ioctls != NULL)
- free(fcaps->fc_ioctls, M_TEMP);
+ free(fcaps->fc_ioctls, M_TEMP);
bzero(fcaps, sizeof(*fcaps));
}
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index 0f4ef12..ecb3a6b 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -228,10 +228,8 @@ sys_cap_rights_limit(struct thread *td, struct cap_rights_limit_args *uap)
if (error == 0) {
fdp->fd_ofiles[fd].fde_rights = rights;
if ((rights & CAP_IOCTL) == 0) {
- if (fdp->fd_ofiles[fd].fde_ioctls != NULL) {
- free(fdp->fd_ofiles[fd].fde_ioctls, M_TEMP);
- fdp->fd_ofiles[fd].fde_ioctls = NULL;
- }
+ free(fdp->fd_ofiles[fd].fde_ioctls, M_TEMP);
+ fdp->fd_ofiles[fd].fde_ioctls = NULL;
fdp->fd_ofiles[fd].fde_nioctls = 0;
}
if ((rights & CAP_FCNTL) == 0 &&
@@ -376,8 +374,7 @@ sys_cap_ioctls_limit(struct thread *td, struct cap_ioctls_limit_args *uap)
FILEDESC_XUNLOCK(fdp);
- if (ocmds != NULL)
- free(ocmds, M_TEMP);
+ free(ocmds, M_TEMP);
return (0);
}
@@ -556,10 +553,8 @@ sys_cap_new(struct thread *td, struct cap_new_args *uap)
*/
fdp->fd_ofiles[newfd].fde_rights = rights;
if ((rights & CAP_IOCTL) == 0) {
- if (fdp->fd_ofiles[newfd].fde_ioctls != NULL) {
- free(fdp->fd_ofiles[newfd].fde_ioctls, M_TEMP);
- fdp->fd_ofiles[newfd].fde_ioctls = NULL;
- }
+ free(fdp->fd_ofiles[newfd].fde_ioctls, M_TEMP);
+ fdp->fd_ofiles[newfd].fde_ioctls = NULL;
fdp->fd_ofiles[newfd].fde_nioctls = 0;
}
if ((rights & CAP_FCNTL) == 0 && fdp->fd_ofiles[newfd].fde_fcntls != 0)
--
1.8.1.3
-------------- next part --------------
From 41fc1f39a18331f57e6a03b54f4b78cbbd123cd5 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:41:38 +0100
Subject: [PATCH 13/24] Remove redundant test.
if needrights is 0, the inner tests will succeed.
---
sys/kern/kern_descrip.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 53b24b1..d1465a0 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -2270,15 +2270,13 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t needrights,
return (EBADF);
#ifdef CAPABILITIES
haverights = cap_rights(fdp, fd);
- if (needrights != 0) {
- error = cap_check(haverights, needrights);
+ error = cap_check(haverights, needrights);
+ if (error != 0)
+ return (error);
+ if ((needrights & CAP_FCNTL) != 0) {
+ error = cap_fcntl_check(fdp, fd, needfcntl);
if (error != 0)
return (error);
- if ((needrights & CAP_FCNTL) != 0) {
- error = cap_fcntl_check(fdp, fd, needfcntl);
- if (error != 0)
- return (error);
- }
}
#endif
count = fp->f_count;
--
1.8.1.3
-------------- next part --------------
From d4c8e6bbeb678867b2bd1c0ec09faf60642465d0 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:43:28 +0100
Subject: [PATCH 14/24] Avoide double test.
---
sys/kern/sys_capability.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index ecb3a6b..dbbda01 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -314,12 +314,12 @@ cap_ioctl_limit_check(struct filedesc *fdp, int fd, const u_long *cmds,
ocmds = fdp->fd_ofiles[fd].fde_ioctls;
for (i = 0; i < ncmds; i++) {
- for (j = 0; j < oncmds; j++) {
+ for (j = 0;; j++) {
+ if (j == oncmds)
+ return (ENOTCAPABLE);
if (cmds[i] == ocmds[j])
break;
}
- if (j == oncmds)
- return (ENOTCAPABLE);
}
return (0);
--
1.8.1.3
-------------- next part --------------
From ba10274ab48bf9b2a2e6b8d50c59eb24074e6ec0 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:45:09 +0100
Subject: [PATCH 15/24] Simplify if (x != 0) x = 0; to just x = 0;.
---
sys/kern/sys_capability.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index dbbda01..153364b 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -232,10 +232,8 @@ sys_cap_rights_limit(struct thread *td, struct cap_rights_limit_args *uap)
fdp->fd_ofiles[fd].fde_ioctls = NULL;
fdp->fd_ofiles[fd].fde_nioctls = 0;
}
- if ((rights & CAP_FCNTL) == 0 &&
- fdp->fd_ofiles[fd].fde_fcntls != 0) {
+ if ((rights & CAP_FCNTL) == 0)
fdp->fd_ofiles[fd].fde_fcntls = 0;
- }
}
FILEDESC_XUNLOCK(fdp);
return (error);
@@ -557,7 +555,7 @@ sys_cap_new(struct thread *td, struct cap_new_args *uap)
fdp->fd_ofiles[newfd].fde_ioctls = NULL;
fdp->fd_ofiles[newfd].fde_nioctls = 0;
}
- if ((rights & CAP_FCNTL) == 0 && fdp->fd_ofiles[newfd].fde_fcntls != 0)
+ if ((rights & CAP_FCNTL) == 0)
fdp->fd_ofiles[newfd].fde_fcntls = 0;
FILEDESC_XUNLOCK(fdp);
--
1.8.1.3
-------------- next part --------------
From 38ec55338714e68b845ee45272fac7e7333727ef Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:47:23 +0100
Subject: [PATCH 16/24] Reduce indentation.
---
sys/kern/kern_descrip.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index d1465a0..612d51a 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -2463,15 +2463,13 @@ fgetvp_rights(struct thread *td, int fd, cap_rights_t need,
if (error != 0)
return (error);
- if (fp->f_vnode == NULL) {
- error = EINVAL;
- } else {
- *vpp = fp->f_vnode;
- vref(*vpp);
- filecaps_copy(&fdp->fd_ofiles[fd].fde_caps, havecaps);
- }
+ if (fp->f_vnode == NULL)
+ return EINVAL;
- return (error);
+ *vpp = fp->f_vnode;
+ vref(*vpp);
+ filecaps_copy(&fdp->fd_ofiles[fd].fde_caps, havecaps);
+ return (0);
}
int
--
1.8.1.3
-------------- next part --------------
From 64e66277c265b3a1221d9e269c8f33345e43332c Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 13:49:37 +0100
Subject: [PATCH 17/24] Avoid code duplication on error paths.
---
sys/kern/sys_capability.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index 153364b..e87a4e5 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -344,37 +344,32 @@ sys_cap_ioctls_limit(struct thread *td, struct cap_ioctls_limit_args *uap)
} else {
cmds = malloc(sizeof(cmds[0]) * ncmds, M_TEMP, M_WAITOK);
error = copyin(uap->cmds, cmds, sizeof(cmds[0]) * ncmds);
- if (error != 0) {
- free(cmds, M_TEMP);
- return (error);
- }
+ if (error != 0)
+ goto out;
}
fdp = td->td_proc->p_fd;
FILEDESC_XLOCK(fdp);
if (fget_locked(fdp, fd) == NULL) {
- FILEDESC_XUNLOCK(fdp);
- free(cmds, M_TEMP);
- return (EBADF);
+ error = EBADF;
+ goto out_locked;
}
error = cap_ioctl_limit_check(fdp, fd, cmds, ncmds);
- if (error != 0) {
- FILEDESC_XUNLOCK(fdp);
- free(cmds, M_TEMP);
- return (error);
- }
+ if (error != 0)
+ goto out_locked;
ocmds = fdp->fd_ofiles[fd].fde_ioctls;
fdp->fd_ofiles[fd].fde_ioctls = cmds;
fdp->fd_ofiles[fd].fde_nioctls = ncmds;
+ cmds = ocmds;
+out_locked:
FILEDESC_XUNLOCK(fdp);
-
- free(ocmds, M_TEMP);
-
- return (0);
+out:
+ free(cmds, M_TEMP);
+ return (error);
}
int
@@ -396,8 +391,8 @@ sys_cap_ioctls_get(struct thread *td, struct cap_ioctls_get_args *uap)
FILEDESC_SLOCK(fdp);
if (fget_locked(fdp, fd) == NULL) {
- FILEDESC_SUNLOCK(fdp);
- return (EBADF);
+ error = EBADF;
+ goto out;
}
/*
@@ -410,19 +405,18 @@ sys_cap_ioctls_get(struct thread *td, struct cap_ioctls_get_args *uap)
if (cmds != NULL && fdep->fde_ioctls != NULL) {
error = copyout(fdep->fde_ioctls, cmds,
sizeof(cmds[0]) * MIN(fdep->fde_nioctls, maxcmds));
- if (error != 0) {
- FILEDESC_SUNLOCK(fdp);
- return (error);
- }
+ if (error != 0)
+ goto out;
}
if (fdep->fde_nioctls == -1)
td->td_retval[0] = INT_MAX;
else
td->td_retval[0] = fdep->fde_nioctls;
+ error = 0;
+out:
FILEDESC_SUNLOCK(fdp);
-
- return (0);
+ return (error);
}
/*
--
1.8.1.3
-------------- next part --------------
From 0e692e05accd56a4db1f938ad7aa4c50979a6148 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 14:00:01 +0100
Subject: [PATCH 18/24] (Hopefully) slightly improve manpages and comments.
---
lib/libc/gen/cap_sandboxed.3 | 6 +++---
lib/libc/sys/cap_fcntls_limit.2 | 14 +++++++-------
lib/libc/sys/cap_ioctls_limit.2 | 24 ++++++++++++------------
lib/libc/sys/cap_rights_limit.2 | 10 +++++-----
sys/sys/capability.h | 4 ++--
5 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/lib/libc/gen/cap_sandboxed.3 b/lib/libc/gen/cap_sandboxed.3
index 4896e02..067d6d2 100644
--- a/lib/libc/gen/cap_sandboxed.3
+++ b/lib/libc/gen/cap_sandboxed.3
@@ -44,10 +44,10 @@
.Fn cap_sandboxed
returns
.Va true
-is the process is in a capability mode sandbox or
+if the process is in a capability mode sandbox or
.Va false
if it is not.
-This function is more handy alternative to the
+This function is a more handy alternative to the
.Xr cap_getmode 2
system call as it always succeeds, so there is no need for error checking.
If the support for capability mode is not compiled into the kernel,
@@ -57,7 +57,7 @@ will always return
.Sh RETURN VALUES
Function
.Fn cap_sandboxed
-is always successful and can return either
+is always successful and will return either
.Va true
or
.Va false .
diff --git a/lib/libc/sys/cap_fcntls_limit.2 b/lib/libc/sys/cap_fcntls_limit.2
index 38ab9cb..8fa7463 100644
--- a/lib/libc/sys/cap_fcntls_limit.2
+++ b/lib/libc/sys/cap_fcntls_limit.2
@@ -44,15 +44,15 @@
.Ft int
.Fn cap_fcntls_get "int fd" "uint32_t *fcntlrightsp"
.Sh DESCRIPTION
-If a file descriptor is granted
+If a file descriptor is granted the
.Dv CAP_FCNTL
-capability right, list of allowed
+capability right, the list of allowed
.Xr fcntl 2
commands can be selectively reduced (but never expanded) with the
.Fn cap_fcntls_limit
system call.
.Pp
-Bitmask of allowed fcntls commands for a given file descriptor can be obtained
+A bitmask of allowed fcntls commands for a given file descriptor can be obtained
with the
.Fn cap_fcntls_get
system call.
@@ -89,13 +89,13 @@ succeeds unless:
.It Bq Er EBADF
The
.Fa fd
-argument is not a valid active descriptor.
+argument is not a valid descriptor.
.It Bq Er EINVAL
An invalid flag has been passed in
.Fa fcntlrights .
.It Bq Er ENOTCAPABLE
.Fa fcntlrights
-would expand list of allowed
+would expand the list of allowed
.Xr fcntl 2
commands.
.El
@@ -106,11 +106,11 @@ succeeds unless:
.It Bq Er EBADF
The
.Fa fd
-argument is not a valid active descriptor.
+argument is not a valid descriptor.
.It Bq Er EFAULT
The
.Fa fcntlrightsp
-argument points at invalid address.
+argument points at an invalid address.
.El
.Sh SEE ALSO
.Xr cap_ioctls_limit 2 ,
diff --git a/lib/libc/sys/cap_ioctls_limit.2 b/lib/libc/sys/cap_ioctls_limit.2
index 34a9bff..de524d7 100644
--- a/lib/libc/sys/cap_ioctls_limit.2
+++ b/lib/libc/sys/cap_ioctls_limit.2
@@ -44,9 +44,9 @@
.Ft ssize_t
.Fn cap_ioctls_get "int fd" "unsigned long *cmds" "size_t maxcmds"
.Sh DESCRIPTION
-If a file descriptor is granted
+If a file descriptor is granted the
.Dv CAP_IOCTL
-capability right, list of allowed
+capability right, the list of allowed
.Xr ioctl 2
commands can be selectively reduced (but never expanded) with the
.Fn cap_ioctls_limit
@@ -62,21 +62,21 @@ There might be up to
.Va 256
elements in the array.
.Pp
-List of allowed ioctl commands for a given file descriptor can be obtained
+The list of allowed ioctl commands for a given file descriptor can be obtained
with the
.Fn cap_ioctls_get
system call.
The
.Fa cmds
-arguments points at the memory that can hold up to
+argument points at memory that can hold up to
.Fa maxcmds
values.
-The function populates provided buffer with up to
+The function populates the provided buffer with up to
.Fa maxcmds
-elements, but always returns total number of ioctl commands allowed for the
+elements, but always returns the total number of ioctl commands allowed for the
given file descriptor.
-Total number of ioctls commands for the given file descriptor can be obtained
-by passing
+The total number of ioctls commands for the given file descriptor can be
+obtained by passing
.Dv NULL as the
.Fa cmds
argument and
@@ -114,11 +114,11 @@ succeeds unless:
.It Bq Er EBADF
The
.Fa fd
-argument is not a valid active descriptor.
+argument is not a valid descriptor.
.It Bq Er EFAULT
The
.Fa cmds
-argument points at invalid address.
+argument points at an invalid address.
.It Bq Er EINVAL
The
.Fa ncmds
@@ -126,7 +126,7 @@ argument is greater than
.Va 256 .
.It Bq Er ENOTCAPABLE
.Fa cmds
-would expand list of allowed
+would expand the list of allowed
.Xr ioctl 2
commands.
.El
@@ -137,7 +137,7 @@ succeeds unless:
.It Bq Er EBADF
The
.Fa fd
-argument is not a valid active descriptor.
+argument is not a valid descriptor.
.It Bq Er EFAULT
The
.Fa cmds
diff --git a/lib/libc/sys/cap_rights_limit.2 b/lib/libc/sys/cap_rights_limit.2
index 2e83ea0..e2ff134 100644
--- a/lib/libc/sys/cap_rights_limit.2
+++ b/lib/libc/sys/cap_rights_limit.2
@@ -68,7 +68,7 @@ Once capability rights are reduced, operations on the file descriptor will be
limited to those permitted by
.Fa rights .
.Pp
-Bitmask of capability rights assigned to a file descriptor can be obtained with
+A bitmask of capability rights assigned to a file descriptor can be obtained with
the
.Fn cap_rights_get
system call.
@@ -178,7 +178,7 @@ Note that only the
and
.Dv F_SETOWN
commands require this capability right.
-Also note that the list of permitted commands can be futher limited with the
+Also note that the list of permitted commands can be further limited with the
.Xr cap_fcntls_limit 2
system call.
.It Dv CAP_FLOCK
@@ -254,7 +254,7 @@ Permit
.Xr ioctl 2 .
Be aware that this system call has enormous scope, including potentially
global scope for some objects.
-The list of permitted ioctl commands can be futher limited with the
+The list of permitted ioctl commands can be further limited with the
.Xr cap_ioctls_limit 2
system call.
.\" XXXPJD: Doesn't exist anymore.
@@ -470,7 +470,7 @@ with the
.Dv O_WRONLY
flag, but without the
.Dv O_APPEND
-flag
+flag,
.Dv CAP_SEEK
is also required.
.El
@@ -503,7 +503,7 @@ argument is not a valid active descriptor.
.It Bq Er EFAULT
The
.Fa rightsp
-argument points at invalid address.
+argument points at an invalid address.
.El
.Sh SEE ALSO
.Xr accept 2 ,
diff --git a/sys/sys/capability.h b/sys/sys/capability.h
index 9eed4d1..130d200 100644
--- a/sys/sys/capability.h
+++ b/sys/sys/capability.h
@@ -267,7 +267,7 @@ int cap_enter(void);
/*
* Are we sandboxed (in capability mode)?
- * This is libc wrapper around cap_getmode(2) system call.
+ * This is a libc wrapper around the cap_getmode(2) system call.
*/
_Bool cap_sandboxed(void);
@@ -289,7 +289,7 @@ int cap_rights_get(int fd, cap_rights_t *rightsp);
*/
int cap_ioctls_limit(int fd, const unsigned long *cmds, size_t ncmds);
/*
- * Returns array of allowed ioctls for the given descriptors.
+ * Returns array of allowed ioctls for the given descriptor.
* If all ioctls are allowed, the cmds array is not populated and
* the function returns INT_MAX.
*/
--
1.8.1.3
-------------- next part --------------
From 2b8a154312f692912fc90bc2acd8e55ab1fc4ba4 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 14:00:31 +0100
Subject: [PATCH 19/24] Sort Xr.
---
lib/libc/sys/cap_rights_limit.2 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/libc/sys/cap_rights_limit.2 b/lib/libc/sys/cap_rights_limit.2
index e2ff134..d8d8777 100644
--- a/lib/libc/sys/cap_rights_limit.2
+++ b/lib/libc/sys/cap_rights_limit.2
@@ -546,8 +546,8 @@ argument points at an invalid address.
.Xr mq_open 2 ,
.Xr open 2 ,
.Xr openat 2 ,
-.Xr pdgetpid 2 ,
.Xr pdfork 2 ,
+.Xr pdgetpid 2 ,
.Xr pdkill 2 ,
.Xr pdwait4 2 ,
.Xr pipe 2 ,
--
1.8.1.3
-------------- next part --------------
From 42aaaa14c5c1a5d96503a88a381909d6c3254727 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 14:04:36 +0100
Subject: [PATCH 20/24] Avoid comparing signed with unsigned values.
---
sys/fs/nfsserver/nfs_nfsdport.c | 2 +-
sys/kern/sys_capability.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c
index 880f965..69f5629 100644
--- a/sys/fs/nfsserver/nfs_nfsdport.c
+++ b/sys/fs/nfsserver/nfs_nfsdport.c
@@ -2775,7 +2775,7 @@ fp_getfvp(struct thread *p, int fd, struct file **fpp, struct vnode **vpp)
int error = 0;
fdp = p->td_proc->p_fd;
- if ((u_int)fd >= fdp->fd_nfiles ||
+ if (fd < 0 || fdp->fd_nfiles <= fd ||
(fp = fdp->fd_ofiles[fd].fde_file) == NULL) {
error = EBADF;
goto out;
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index e87a4e5..e3622b0 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -274,7 +274,7 @@ cap_ioctl_check(struct filedesc *fdp, int fd, u_long cmd)
{
u_long *cmds;
ssize_t ncmds;
- u_int i;
+ ssize_t i;
FILEDESC_LOCK_ASSERT(fdp);
KASSERT(fd >= 0 && fd < fdp->fd_nfiles,
@@ -302,12 +302,13 @@ cap_ioctl_limit_check(struct filedesc *fdp, int fd, const u_long *cmds,
{
u_long *ocmds;
ssize_t oncmds;
- u_int i, j;
+ size_t i;
+ ssize_t j;
oncmds = fdp->fd_ofiles[fd].fde_nioctls;
if (oncmds == -1)
return (0);
- if (oncmds < ncmds)
+ if ((size_t)oncmds < ncmds)
return (ENOTCAPABLE);
ocmds = fdp->fd_ofiles[fd].fde_ioctls;
--
1.8.1.3
-------------- next part --------------
From a05144e19411d2b3fc8716e72ef2fad7cc9449ae Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 15:09:16 +0100
Subject: [PATCH 21/24] For readability simplify if (x) y = a; else y = b; with
long y to y = x ? a : b.
---
sys/kern/kern_descrip.c | 10 +++-------
sys/kern/sys_capability.c | 6 ++----
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 612d51a..30dac04 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -842,13 +842,9 @@ do_dup(struct thread *td, int flags, int old, int new,
fdp->fd_ofiles[new] = fdp->fd_ofiles[old];
filecaps_copy(&fdp->fd_ofiles[old].fde_caps,
&fdp->fd_ofiles[new].fde_caps);
- if ((flags & DUP_CLOEXEC) != 0) {
- fdp->fd_ofiles[new].fde_flags =
- fdp->fd_ofiles[old].fde_flags | UF_EXCLOSE;
- } else {
- fdp->fd_ofiles[new].fde_flags =
- fdp->fd_ofiles[old].fde_flags & ~UF_EXCLOSE;
- }
+ fdp->fd_ofiles[new].fde_flags = flags & DUP_CLOEXEC ?
+ fdp->fd_ofiles[old].fde_flags | UF_EXCLOSE :
+ fdp->fd_ofiles[old].fde_flags & ~UF_EXCLOSE;
if (new > fdp->fd_lastfile)
fdp->fd_lastfile = new;
*retval = new;
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index e3622b0..2306811 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -409,10 +409,8 @@ sys_cap_ioctls_get(struct thread *td, struct cap_ioctls_get_args *uap)
if (error != 0)
goto out;
}
- if (fdep->fde_nioctls == -1)
- td->td_retval[0] = INT_MAX;
- else
- td->td_retval[0] = fdep->fde_nioctls;
+ td->td_retval[0] =
+ fdep->fde_nioctls != -1 ? fdep->fde_nioctls : INT_MAX;
error = 0;
out:
--
1.8.1.3
-------------- next part --------------
From 1258951ef3cdb8b8624e6a7032036e0e5e0ac8c6 Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 15:23:15 +0100
Subject: [PATCH 22/24] Unify and simplify bitset inclusion tests.
- (have | need) != have looks like a typo: shouldn't the | be a &?.
- some places used (have | need) != have, others (have & need) == need.
- (need & ~have) != 0 -- need and not have -- is easier to comprehend.
- Avoid duplication, especially when the duplicated subexpression is long.
---
sys/kern/kern_descrip.c | 4 ++--
sys/kern/sys_capability.c | 11 +++++------
usr.bin/procstat/procstat_files.c | 4 ++--
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 30dac04..64176ca 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1422,9 +1422,9 @@ static void
filecaps_validate(const struct filecaps *fcaps, const char *func)
{
- KASSERT((fcaps->fc_rights | CAP_MASK_VALID) == CAP_MASK_VALID,
+ KASSERT((fcaps->fc_rights & ~CAP_MASK_VALID) == 0,
("%s: invalid rights", func));
- KASSERT((fcaps->fc_fcntls | CAP_FCNTL_ALL) == CAP_FCNTL_ALL,
+ KASSERT((fcaps->fc_fcntls & ~CAP_FCNTL_ALL) == 0,
("%s: invalid fcntls", func));
KASSERT(fcaps->fc_fcntls == 0 || (fcaps->fc_rights & CAP_FCNTL) != 0,
("%s: fcntls without CAP_FCNTL", func));
diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c
index 2306811..d06918c 100644
--- a/sys/kern/sys_capability.c
+++ b/sys/kern/sys_capability.c
@@ -148,7 +148,7 @@ static inline int
_cap_check(cap_rights_t have, cap_rights_t need, enum ktr_cap_fail_type type)
{
- if ((have | need) != have) {
+ if ((need & ~have) != 0) {
#ifdef KTRACE
if (KTRPOINT(curthread, KTR_CAPFAIL))
ktrcapfail(type, need, have);
@@ -215,7 +215,7 @@ sys_cap_rights_limit(struct thread *td, struct cap_rights_limit_args *uap)
AUDIT_ARG_FD(fd);
AUDIT_ARG_RIGHTS(rights);
- if ((rights | CAP_ALL) != CAP_ALL)
+ if ((rights & ~CAP_ALL) != 0)
return (EINVAL);
fdp = td->td_proc->p_fd;
@@ -452,7 +452,7 @@ sys_cap_fcntls_limit(struct thread *td, struct cap_fcntls_limit_args *uap)
AUDIT_ARG_FD(fd);
AUDIT_ARG_FCNTL_RIGHTS(fcntlrights);
- if ((fcntlrights | CAP_FCNTL_ALL) != CAP_FCNTL_ALL)
+ if ((fcntlrights & ~CAP_FCNTL_ALL) != 0)
return (EINVAL);
fdp = td->td_proc->p_fd;
@@ -463,8 +463,7 @@ sys_cap_fcntls_limit(struct thread *td, struct cap_fcntls_limit_args *uap)
return (EBADF);
}
- if ((fdp->fd_ofiles[fd].fde_fcntls | fcntlrights) !=
- fdp->fd_ofiles[fd].fde_fcntls) {
+ if ((fcntlrights & ~fdp->fd_ofiles[fd].fde_fcntls) != 0) {
FILEDESC_XUNLOCK(fdp);
return (ENOTCAPABLE);
}
@@ -515,7 +514,7 @@ sys_cap_new(struct thread *td, struct cap_new_args *uap)
AUDIT_ARG_FD(fd);
AUDIT_ARG_RIGHTS(rights);
- if ((rights | CAP_ALL) != CAP_ALL)
+ if ((rights & ~CAP_ALL) != 0)
return (EINVAL);
fdp = td->td_proc->p_fd;
diff --git a/usr.bin/procstat/procstat_files.c b/usr.bin/procstat/procstat_files.c
index eefc5b7..16dab0b 100644
--- a/usr.bin/procstat/procstat_files.c
+++ b/usr.bin/procstat/procstat_files.c
@@ -243,7 +243,7 @@ width_capability(cap_rights_t rights)
count = 0;
width = 0;
for (i = 0; i < cap_desc_count; i++) {
- if ((rights & cap_desc[i].cd_right) == cap_desc[i].cd_right) {
+ if ((cap_desc[i].cd_right & ~rights) == 0) {
width += strlen(cap_desc[i].cd_desc);
if (count)
width++;
@@ -267,7 +267,7 @@ print_capability(cap_rights_t rights, u_int capwidth)
printf("-");
}
for (i = 0; i < cap_desc_count; i++) {
- if ((rights & cap_desc[i].cd_right) == cap_desc[i].cd_right) {
+ if ((cap_desc[i].cd_right & ~rights) == 0) {
printf("%s%s", count ? "," : "", cap_desc[i].cd_desc);
width += strlen(cap_desc[i].cd_desc);
if (count)
--
1.8.1.3
-------------- next part --------------
From 4d721b7aa909091fc705cc8f822a13da69e1954c Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 15:33:48 +0100
Subject: [PATCH 23/24] Simplify assertion condition, which contains a
duplicated subexpression.
---
sys/kern/kern_descrip.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 64176ca..c2556e2 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1428,9 +1428,8 @@ filecaps_validate(const struct filecaps *fcaps, const char *func)
("%s: invalid fcntls", func));
KASSERT(fcaps->fc_fcntls == 0 || (fcaps->fc_rights & CAP_FCNTL) != 0,
("%s: fcntls without CAP_FCNTL", func));
- KASSERT(((fcaps->fc_nioctls == -1 || fcaps->fc_nioctls == 0) &&
- fcaps->fc_ioctls == NULL) ||
- (fcaps->fc_nioctls > 0 && fcaps->fc_ioctls != NULL),
+ KASSERT(fcaps->fc_ioctls != NULL ? fcaps->fc_nioctls > 0 :
+ (fcaps->fc_nioctls == -1 || fcaps->fc_nioctls == 0),
("%s: invalid ioctls", func));
KASSERT(fcaps->fc_nioctls == 0 || (fcaps->fc_rights & CAP_IOCTL) != 0,
("%s: ioctls without CAP_IOCTL", func));
--
1.8.1.3
-------------- next part --------------
From 98dd3c8a988e043b4a8f02ed4c19d39d30e1145d Mon Sep 17 00:00:00 2001
From: Christoph Mallon <christoph.mallon at gmx.de>
Date: Sun, 24 Feb 2013 15:47:53 +0100
Subject: [PATCH 24/24] Factorise code to free a file descriptor.
---
sys/kern/kern_descrip.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index c2556e2..8256501 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -287,6 +287,19 @@ fdunused(struct filedesc *fdp, int fd)
}
/*
+ * Free a file descriptor.
+ */
+static void
+fdfree(struct filedesc *fdp, int fd)
+{
+ struct filedescent *fde = fdp->fd_ofiles[fd];
+
+ filecaps_free(&fde->fde_caps);
+ bzero(fde, sizeof(*fde));
+ fdunused(fdp, fd);
+}
+
+/*
* System calls on descriptors.
*/
#ifndef _SYS_SYSPROTO_H_
@@ -1165,9 +1178,7 @@ kern_close(td, fd)
FILEDESC_XUNLOCK(fdp);
return (EBADF);
}
- filecaps_free(&fdp->fd_ofiles[fd].fde_caps);
- bzero(&fdp->fd_ofiles[fd], sizeof(fdp->fd_ofiles[fd]));
- fdunused(fdp, fd);
+ fdfree(fdp, fd);
/* closefp() drops the FILEDESC lock for us. */
return (closefp(fdp, fd, fp, td, 1));
@@ -2035,9 +2046,7 @@ setugidsafety(struct thread *td)
* NULL-out descriptor prior to close to avoid
* a race while close blocks.
*/
- filecaps_free(&fdp->fd_ofiles[i].fde_caps);
- bzero(&fdp->fd_ofiles[i], sizeof(fdp->fd_ofiles[i]));
- fdunused(fdp, i);
+ fdfree(fdp, i);
FILEDESC_XUNLOCK(fdp);
(void) closef(fp, td);
FILEDESC_XLOCK(fdp);
@@ -2059,9 +2068,7 @@ fdclose(struct filedesc *fdp, struct file *fp, int idx, struct thread *td)
FILEDESC_XLOCK(fdp);
if (fdp->fd_ofiles[idx].fde_file == fp) {
- filecaps_free(&fdp->fd_ofiles[idx].fde_caps);
- bzero(&fdp->fd_ofiles[idx], sizeof(fdp->fd_ofiles[idx]));
- fdunused(fdp, idx);
+ fdfree(fdp, idx);
FILEDESC_XUNLOCK(fdp);
fdrop(fp, td);
} else
@@ -2094,9 +2101,7 @@ fdcloseexec(struct thread *td)
fp = fde->fde_file;
if (fp != NULL && (fp->f_type == DTYPE_MQUEUE ||
(fde->fde_flags & UF_EXCLOSE))) {
- filecaps_free(&fde->fde_caps);
- bzero(fde, sizeof(*fde));
- fdunused(fdp, i);
+ fdfree(fdp, i);
(void) closefp(fdp, i, fp, td, 0);
/* closefp() drops the FILEDESC lock. */
FILEDESC_XLOCK(fdp);
--
1.8.1.3
More information about the freebsd-arch
mailing list