I've made a quick pass through your patch and written a few notes about areas of concern from my perspective. Sorry if I misrepresent your patch in some way - this was done hastily. I hope to make a more careful review after returning from the USENIX Annual Technical Conference. Here are my notes: Your patch moves various portions of the base kernel logic into the hook functions. The direction on the mailing list seemed to be the opposite - leaving the base kernel logic untouched, and only moving the core capability logic into hook functions (not calls to capable). Unless you can obtain buy-in from the kernel developers a priori for moving the base kernel logic into the hooks, I don't think we want to go down this road. (And even with buy-in, most of us don't want to deal with this task - it doesn't help most of us with our security modules). Some of your hook calls pass filenames provided by applications. To be fair, this was already true for the task set_label hook, but I also question the need for that hook given the binprm security operations. I would argue that the hooks should be passed the result of the kernel lookup, i.e. the struct dentry (and vfsmount if you really need the absolute pathname) or the struct file. Otherwise, there is a race condition between the module lookup of the pathname and the base kernel lookup of the pathname, and the module must deal with things like relative pathnames. Your patch introduces a lot of hooks for capturing final return statuses. Why can't this be provided by a module simply by interposing on the system calls like any existing LKM and capturing the final result in that manner? Why must we add explicit hooks into the base kernel for this purpose? Your patch changes the file_ops to take fd's rather than struct file *'s. That's bad for us (SELinux) - it means we have to make another lookup of the fd, even though the kernel already has performed the lookup. Again, an unnecessary race condition. Why do you care about fd values - what useful information can be recovered from an old fd value in an audit log? Given the struct file, you can easily generate the pathname - isn't that more useful? Your patch moves the mmap checks from the internal function do_mmap_pgoff, which is called by do_mmap, which is called by the binary loaders in fs/binfmt*.c. So with your mmap hook placement, we must duplicate the mmap checks in our execve-related hooks. Furthermore, your mmap hook placement requires changes to every architecture, whereas our hook placement provides centralization. -- Stephen D. Smalley, NAI Labs ssmalleyat_private _______________________________________________ linux-security-module mailing list linux-security-moduleat_private http://mail.wirex.com/mailman/listinfo/linux-security-module
This archive was generated by hypermail 2b30 : Wed Jun 27 2001 - 08:27:33 PDT