10:09 a.m.
On Wednesday 27 April 2005 19:17, Timothy R. Chavez wrote:
diff -Nurp linux-2.6.12-rc2-mm1~orig/include/linux/audit.h
linux-2.6.12-rc2-mm1~audit/include/linux/audit.h ---
linux-2.6.12-rc2-mm1~orig/include/linux/audit.h 2005-04-11
09:15:03.000000000 -0500 +++
linux-2.6.12-rc2-mm1~audit/include/linux/audit.h 2005-04-27
16:37:54.000000000 -0500 @@ -24,18 +24,27 @@
#ifndef _LINUX_AUDIT_H_
#define _LINUX_AUDIT_H_
+#ifdef __KERNEL__
#include <linux/sched.h>
#include <linux/elf.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <asm/atomic.h>
+#endif
Do you need these headers or just the forward declaration for the data types
used in this header?
+struct audit_data {
+ struct audit_wentry *wentry;
+ struct hlist_head watchlist;
+ rwlock_t lock;
+};
audit_data seems very generic. Like something the core auditing system would
have. It would be more meaningful if it were named something related to the
file system or watch. audit_wdata?
@@ -190,6 +236,7 @@ extern void audit_get_stamp(struct audit
extern int audit_set_loginuid(struct audit_context *ctx, uid_t loginuid);
extern uid_t audit_get_loginuid(struct audit_context *ctx);
extern int audit_ipc_perms(unsigned long qbytes, uid_t uid, gid_t gid,
mode_t
mode);
+extern int audit_notify_watch(struct inode *inode, int mask);
#else
#define audit_alloc(t) ({ 0; })
#define audit_free(t) do { ; } while (0)
I wonder if this function declaration should have a #ifdef
CONFIG_AUDITFILESYSTEM around it and then use the macro if not. Same goes for
the function definition in auditsc.c.
(audit.c)
@@ -416,6 +419,17 @@ static int audit_receive_msg(struct sk_b
err = -EOPNOTSUPP;
#endif
break;
+ case AUDIT_WATCH_LIST:
+ err = audit_list_watches(pid, seq);
+ break;
+ case AUDIT_WATCH_INS:
+ case AUDIT_WATCH_REM:
+ if (nlh->nlmsg_len < sizeof(struct audit_transport))
+ return -EINVAL;
+ err = audit_receive_watch(nlh->nlmsg_type,
+ NETLINK_CB(skb).pid,
Should the call to audit_list_watches be surrounded by #ifdef
CONFIG_AUDITFILESYSTEM and then return -EOPNOTSUPP in the #else? Same with
audit_receive_watch.
Looking through the code for auditfs.c, there is no use of the macros likely
and unlikely to help branch prediction. I'll throw this out to Chris & David,
do we have any critical paths that would benefit from this code being
augmented with those macros?
(auditfs.c)
+ /* Payload */
+ memcpy(memblk, &req, sizeof(req));
+ offset = total - req.fklen;
+ memcpy(memblk + offset, watch->filterkey, req.fklen);
+ offset = offset - req.pathlen;
+ memcpy(memblk + offset, watch->path, req.pathlen);
Are these offsets being calculated right?
Seems like sb
+ memcpy(memblk, &req, sizeof(req));
+ offset = sizeof(req);
+ memcpy(memblk + offset, watch->filterkey, req.fklen);
+ offset += req.fklen;
+ memcpy(memblk + offset, watch->path, req.pathlen)
?
+int audit_receive_watch(int type, int pid, int uid, int seq,
+ struct audit_transport *req)
+{
+ int ret = 0;
+ char *path = NULL;
+ char *filterkey = NULL;
+
+ ret = -ENOMEM;
+ path = kmalloc(req->pathlen, GFP_KERNEL);
+ if (!path)
+ goto audit_receive_watch_exit;
+ strncpy(path, req->buf, req->pathlen);
+
+ if (req->fklen) {
+ filterkey = kmalloc(req->fklen, GFP_KERNEL);
+ if (!filterkey)
+ goto audit_receive_watch_exit;
+ }
+ strncpy(filterkey, req->buf+req->pathlen, req->fklen);
+
+ ret = -EINVAL;
+ if (!path || strlen(path) + 1 > PATH_MAX)
+ goto audit_receive_watch_exit;
+
+ /* Includes terminating '\0' */
+ if (req->fklen > AUDIT_FILTERKEY_MAX)
+ goto audit_receive_watch_exit;
+
+ if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND))
+ goto audit_receive_watch_exit;
Whatever happened to checking parameters and then allocating memory? I thought
this was done right a few revs ago.
+int audit_notify_watch(struct inode *inode, int mask)
+{
+ int ret = 0;
+ struct audit_context *context = current->audit_context;
+ struct audit_aux_data_watched *ax;
+ struct audit_wentry *wentry = NULL;
+
+ if (likely(!context))
+ goto audit_notify_watch_fail;
Question. If there is no context, is there a possibility of generating one?
Under what circumstances besides no memory would a context not exist?
Based on what I see, I think I can merge the userspace portion for listing
watches. There's cleanups needed, but I think the intent is correct.
-Steve