From: Kylene Jo Hall 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 Signed-off-by: Mimi Zohar Signed-off-by: Andrew Morton --- security/slim/slm_main.c | 29 ++++++++++------------------- 1 files changed, 10 insertions(+), 19 deletions(-) diff -puN security/slim/slm_main.c~slim-main-patch-misc-cleanups-requested-at-inclusion-time security/slim/slm_main.c --- a/security/slim/slm_main.c~slim-main-patch-misc-cleanups-requested-at-inclusion-time +++ a/security/slim/slm_main.c @@ -58,6 +58,7 @@ static char *get_token(char *buf_start, break; case '#': while ((*bufp != '\n') && (bufp++ < buf_end)) ; + bufp++; break; default: @@ -133,7 +134,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; @@ -149,21 +149,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; } @@ -226,7 +217,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; @@ -1056,14 +1047,14 @@ static int slm_task_post_setuid(uid_t ol return 0; } -static inline int slm_setprocattr(struct task_struct *tsk, +static int slm_setprocattr(struct task_struct *tsk, char *name, void *value, size_t size) { return -EACCES; } -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; @@ -1209,7 +1200,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; _