Re: Some feedback on the hooks

gregat_private

I'll just comment on the pieces that Chris didn't.  If I missed
anything, please let me know.

On Wed, Apr 25, 2001 at 03:14:32PM -0400, Stephen Smalley wrote:
> 
> Here is some feedback on the hooks in the current patch.  This
> is by no means a complete assessment, just a start.  We'd be willing to
> assist with implementing some of these proposed changes and additions, but
> you'll have to let us know what areas we can safely work on without
> conflicting with your ongoing changes.

Thank you so much, this is exactly what we are looking for.

> fs/super.c (sys_umount):  Currently, you call the umount hook in
> sys_umount on the name and flags.  We would prefer a hook in do_umount
> on the sb object itself.  Otherwise, we have to repeat the lookup.
> Also, we need a hook to close files during the unmount that are being used
> for persistent label mappings (analagous to the existing DQUOT_OFF
> and acct_auto_close calls for quota files and accounting files).
> Also, you are currently using the user space pointers for the name
> in the call to the hook.  If you are going to keep name-based
> hooks, you probably want to move the hook call and use the kernel's
> kname, since that it is used for the lookup.  In general, I don't
> think we should pass any user pointers to hooks.  

I moved the hook to do_umount, and fixed the userspace issue.  Thanks
for pointing that one out, I agree that no userpointers should be passed
to hooks.
Does the movement of the hook now work for all your cases, or where do
you suggest to place a new one, right before/after DQUOT_OFF?  And if
the umount fails later after this call, do you need to put things back
again?
See the latest patch for these changes.

> fs/super.c (sys_mount):  Again, we would prefer a hook in
> do_remount on the sb object, a hook in do_mount on the mount
> directory's dentry (before reading the superblock of the
> file system to be mounted), and a hook in do_mount on the new
> sb object (after reading the superblock, but before adding
> it to the file system name space).  Same issues with user space
> pointers here too.

I moved the sys_mount check to do_remount.  Since you mention
do_remount, should we also hook there?  Actually if you could give a
small patch snippet of what you are looking for here, would help me out
lots.  Fixed the userspace pointers, thanks.

> kernel/module.c:  User space pointers again.  The hooks should
> be called with the same kernel copy of the data that gets used
> later for the actual operation.

Got it, thanks.

Hm, need to go cross-check your other comments against the code, I'll
respond tomorrow.

thanks again,

greg k-h

_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module