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

On Monday 14 March 2005 18:14, Timothy R. Chavez wrote: 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