Re: [RFC][PATCH] auditfs userspace

On Tuesday 08 March 2005 10:14, Timothy R. Chavez wrote: I don't think its a good idea to mirror them. We'll move them to the right place and fix the includes. But there's still a question from the last note...do we need a fs_enable? We don't have a rule enable. Its either got rules and produces output or it doesn't. (Ignore the fact that SE Linux avc messages get sent even with no rules.) In auditsc.c is this chunk of code: + if (!auditfs_enabled) + return; + + list_for_each_entry(file, &context->wtrail, list) { + ab = audit_log_start(context); + if (!ab) + continue; list_for_each_entry should avoid the loop if there's no rules. Therefore its less efficient to add a check for auditfs_enabled. + audit_log_format(ab, + "name=%s filter_key=%s perm_mask=%u perm=%d inode=%lu " + "inode_mode=%d inode_uid=%d inode_gid=%d " + "inode_dev=%02x:%02x inode_rdev=%02x:%02x", + file->wentry->w_watch->name, + file->wentry->w_watch->filterkey, + file->wentry->w_watch->perms, + file->mask, file->ino, file->mode, file->uid, + file->gid, MAJOR(file->dev), MINOR(file->dev), + MAJOR(file->rdev), MINOR(file->rdev)); Are perm_mask & perm variables in the wrong order? I have questions about audit_wentry_free - shouldn't wentry be zero'ed when its free instead of pointing to freed memory? I see a lot of code that branches if wentry is non-zero. Is the way freeing works racy? Is the list locked (I think I see 2 different locks)? Can other threads try to reference a partially deleted audit_wentry? What about the name audit_wentry_put? How about audit_wentry_decr_ref? put sounds like a name that is inserting things, not potentially freeing them. Since it sometimes frees things, doesn't this leak memory: + audit_wentry_put(data->wentry); + data->wentry = NULL; The zeroing should be atomic and in sync with reference counts. -Steve