[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [RFC][PATCH] (#6) filesystem auditing



On Monday 14 March 2005 18:14, Timothy R. Chavez wrote:
> This patch has enough changes in it to be called patch #6

I just noticed this in audit.h:

struct audit_watch {
       int     namelen;
       int     fklen;

Shouldn't these be pinned down to a byte size? Like __u32 or __u16? It seems 
safer to me when you consider userspace to kernel packets and whether the 
kernel is 64 bit and userspace 32 bit.

But more interesting...what if a -1 was sent for fklen?

+       if (req->fklen) {
+               ret = -ENOMEM;
+               filterkey = kmalloc(req->fklen, GFP_KERNEL);

Kaboom...

Also a nit, you have a structure audit_watch and a function audit_watch.

In audit.c in the audit_receive_msg  function,
@@ -413,6 +416,12 @@ static int audit_receive_msg(struct sk_b
                err = -EOPNOTSUPP;
 #endif
                break;
+       case AUDIT_WATCH_INS:
+       case AUDIT_WATCH_REM:
+               err = audit_receive_watch(nlh->nlmsg_type,
+                                         NETLINK_CB(skb).pid,
+                                         uid, seq, data);
+               break;

Shouldn't there be some checking of the packet like so (may not be 100% right, 
but you should see what I mean):

 if (nlh->nlmsg_len != sizeof(struct audit_watch))
         return -EINVAL

before sending it into audit_receive_watch?  And then shouldn't some reality 
checks be done like making sure no file name is greater than MAX_PATH and the 
filterkey is reasonable size before using them in audit_insert_watch?

Also, how do you list the watches? I was looking in userspace code and only 
see insert and remove, no listing.

That's what I see for now...

-Steve


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]