From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Timothy R. Chavez" Subject: Re: [PATCH] Audit of POSIX Message Queue Syscalls Date: Wed, 17 May 2006 09:34:44 -0500 Message-ID: <1147876484.11589.46.camel@localhost.localdomain> References: <20060517014055.GA16852@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (mx1.redhat.com [172.16.48.31]) by int-mx1.corp.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id k4HEYqhY014901 for ; Wed, 17 May 2006 10:34:52 -0400 Received: from e35.co.us.ibm.com (e35.co.us.ibm.com [32.97.110.153]) by mx1.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id k4HEYoSa014454 for ; Wed, 17 May 2006 10:34:50 -0400 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by e35.co.us.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id k4HEYjKp019460 for ; Wed, 17 May 2006 10:34:45 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay04.boulder.ibm.com (8.12.10/NCO/VER6.8) with ESMTP id k4HEYeBl145650 for ; Wed, 17 May 2006 08:34:45 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.12.11/8.13.3) with ESMTP id k4HEYeae030676 for ; Wed, 17 May 2006 08:34:40 -0600 In-Reply-To: <20060517014055.GA16852@us.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: "George C. Wilson" Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com Hey George, Just some quick things. I didn't want to reproduce any of the things Steve pointed out already... These are just general comments... -tim On Tue, 2006-05-16 at 20:40 -0500, George C. Wilson wrote: > @@ -660,6 +663,9 @@ asmlinkage long sys_mq_open(const char _ > if (IS_ERR(name = getname(u_name))) > return PTR_ERR(name); > > + if ((error = audit_mq_open(oflag, mode, u_attr)) != 0) > + return error; > + > fd = get_unused_fd(); > if (fd < 0) > goto out_putname; > @@ -814,6 +820,9 @@ asmlinkage long sys_mq_timedsend(mqd_t m > long timeout; > int ret; > > + if ((ret = audit_mq_timedsend(mqdes, msg_len, msg_prio, u_abs_timeout)) != 0) > + return ret; > + Take the assignment out of the dang conditional :) Besides, you're over 80 characters here aren't ya? Also, you should probably just follow the same convention in the function.. ret = audit_mg_timedsend(..); if (!ret) goto out; [..] > if (unlikely(msg_prio >= (unsigned long) MQ_PRIO_MAX)) > return -EINVAL; > > @@ -896,6 +905,9 @@ asmlinkage ssize_t sys_mq_timedreceive(m > struct mqueue_inode_info *info; > struct ext_wait_queue wait; > > + if ((ret = audit_mq_timedreceive(mqdes, msg_len, u_msg_prio, u_abs_timeout)) != 0) > + return ret; > + > timeout = prepare_timeout(u_abs_timeout); > > ret = -EBADF; > @@ -975,6 +987,9 @@ asmlinkage long sys_mq_notify(mqd_t mqde > struct mqueue_inode_info *info; > struct sk_buff *nc; > > + if ((ret = audit_mq_notify(mqdes, u_notification)) != 0) > + return ret; > + > nc = NULL; > sock = NULL; > if (u_notification != NULL) { > @@ -1087,7 +1102,7 @@ asmlinkage long sys_mq_getsetattr(mqd_t > const struct mq_attr __user *u_mqstat, > struct mq_attr __user *u_omqstat) > { > - int ret; > + int ret, audret; > struct mq_attr mqstat, omqstat; > struct file *filp; > struct inode *inode; > @@ -1130,9 +1145,13 @@ asmlinkage long sys_mq_getsetattr(mqd_t > sizeof(struct mq_attr))) > ret = -EFAULT; > > + Eh, get rid of these... [..] > out_fput: > fput(filp); > out: > + audret = audit_mq_getsetattr(mqdes, &mqstat, &omqstat); > + if (ret == 0) > + ret = audret; At a cursory glance, this looks a little fishy to me... > return ret; > } >