[PATCH] slim: misc cleanups requested at inclusion time.

[Kylene Jo Hall <kjhall@us.ibm.com>]

Miscellaneous fixes requested when the intial patches were pulled in
last week.  This patch addresses the following comments.


On Thu, 2006-09-14 at 16:52 -0700, Andrew Morton wrote: 
> trivial things:
> > +char *slm_iac_str[] = {
> > +	ZERO_STR,
> > +	UNTRUSTED_STR,
> > +	USER_STR,
> > +	SYSTEM_STR
> > +};
> 
> Could this be made static?
> 
No because it is used in slm_secfs.c.

> > +		case '#':
> > +			while ((*bufp != '\n') && (bufp++ < buf_end)) ;
> 
> newline needed here.
Added in the patch.

> > +static int has_file_wperm(struct slm_file_xattr *cur_level)
> > +{
> > +	int i, j = 0;
> > +	struct files_struct *files = current->files;
> > +	unsigned long fd = 0;
> > +	struct fdtable *fdt;
> > +	struct file *file;
> > +	int rc = 0;
> > +
> > +	if (is_kernel_thread(current))
> > +		return 0;
> > +
> > +	if (!files || !cur_level)
> > +		return 0;
> > +
> > +	spin_lock(&files->file_lock);
> > +	fdt = files_fdtable(files);
> > +
> > +	for (;;) {
> > +		i = j * __NFDBITS;
> > +		if (i >= fdt->max_fdset || i >= fdt->max_fds)
> > +			break;
> > +		fd = fdt->open_fds->fds_bits[j++];
> > +		while (fd) {
> > +			if (fd & 1) {
> > +				file = fdt->fd[i++];
> > +				if (file)
> > +					rc = mark_has_file_wperm(file,
> > +						cur_level);
> > +			}
> > +			fd >>= 1;
> > +		}
> > +	}
> > +	spin_unlock(&files->file_lock);
> > +	return rc;
> > +}
> 
> Could we use a more meaningful identifier than `j' here?  Perhaps `i' too?
This function has been cleaned up with the use of find_next_bit and i
and j are no longer used in the patch below.

> > +	for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
> 
> ARRAY_SIZE()
Fixed below.


> > +		if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
> > +		    == 0)
> > +			return iac;
> > +	}
> > +	return SLM_IAC_ERROR;
> > +}
> > +
> >
> > ...
> >
> > +
> > +static inline int slm_getprocattr(struct task_struct *tsk,
> > +				  char *name, void *value, size_t size)
> It's rather pointless to declare a function inline and to then only refer to
> it via a pointer-to-function.  Please review all inlinings.
All inlines have been reviewed and fixed if necessary in the patch
below.  3 hooks were affected.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
security/slim/slm_main.c |   29 ++++++++++-------------------
1 files changed, 10 insertions(+), 19 deletions(-) 
--- linux-2.6.18-rc6-orig/security/slim/slm_main.c	2006-09-18 16:41:51.000000000 -0500
+++ linux-2.6.18-rc6/security/slim/slm_main.c	2006-09-19 16:45:34.000000000 -0500
@@ -61,6 +60,7 @@ static char *get_token(char *buf_start, 
 			break;
 		case '#':
 			while ((*bufp != '\n') && (bufp++ < buf_end)) ;
+
 			bufp++;
 			break;
 		default:
@@ -141,7 +141,6 @@ static inline int mark_has_file_wperm(st
  */
 static int has_file_wperm(struct slm_file_xattr *cur_level)
 {
-	int i, j = 0;
 	struct files_struct *files = current->files;
 	unsigned long fd = 0;
 	struct fdtable *fdt;
@@ -157,21 +156,12 @@ static int has_file_wperm(struct slm_fil
 	spin_lock(&files->file_lock);
 	fdt = files_fdtable(files);
 
-	for (;;) {
-		i = j * __NFDBITS;
-		if (i >= fdt->max_fdset || i >= fdt->max_fds)
-			break;
-		fd = fdt->open_fds->fds_bits[j++];
-		while (fd) {
-			if (fd & 1) {
-				file = fdt->fd[i++];
-				if (file)
-					rc = mark_has_file_wperm(file,
-						cur_level);
-			}
-			fd >>= 1;
-		}
+	while((fd=find_next_bit(fdt->open_fds->fds_bits, fdt->max_fdset, fd)) < fdt->max_fdset) {
+		file = fdt->fd[fd++];
+		if (file)
+			rc = mark_has_file_wperm(file, cur_level);
 	}
+
 	spin_unlock(&files->file_lock);
 	return rc;
 }
@@ -241,7 +231,7 @@ static enum slm_iac_level parse_iac(char
 
 	if (strncmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
 		return SLM_IAC_EXEMPT;
-	for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
+	for (iac = 0; iac < ARRAY_SIZE(slm_iac_str); iac++) {
 		if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
 		    == 0)
 			return iac;
@@ -1243,7 +1233,7 @@ static int slm_task_post_setuid(uid_t ol
 	return rc;
 }
 
-static inline int slm_setprocattr(struct task_struct *tsk,
+static int slm_setprocattr(struct task_struct *tsk,
 				  char *name, void *value, size_t size)
 {
 	dprintk(SLM_BASE, "%s: %s \n", __FUNCTION__, name);
@@ -1251,7 +1241,7 @@ static inline int slm_setprocattr(struct
 
 }
 
-static inline int slm_getprocattr(struct task_struct *tsk,
+static int slm_getprocattr(struct task_struct *tsk,
 				  char *name, void *value, size_t size)
 {
 	struct slm_tsec_data *tsec = tsk->security;
@@ -1473,7 +1463,7 @@ static int slm_inode_setattr(struct dent
 	return rc;
 }
 
-static inline int slm_capable(struct task_struct *tsk, int cap)
+static int slm_capable(struct task_struct *tsk, int cap)
 {
 	struct slm_tsec_data *tsec = tsk->security;
 	int rc = 0;


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/