Subject: Re: Implementation of POSIX message queue
To: Mindaugas R. <rmind@NetBSD.org>
From: David Holland <dholland+netbsd@eecs.harvard.edu>
List: tech-kern
Date: 08/16/2007 22:09:26
On Fri, Aug 17, 2007 at 03:02:48AM +0300, Mindaugas R. wrote:
 > > 	/*
 > > 	 * According to POSIX - no message should be removed on
 > > 	 * failing case, thus we will check the pointers here.
 > > 	 */
 > > 	if (subyte(msg_ptr, 0) == -1)
 > > 		return EFAULT;
 > > 	if (msg_prio && (subyte(msg_prio, 0) == -1))
 > > 		return EFAULT;
 > > 
 > > There's no real point to this and it's expensive to do. Does it make the
 > > system call pass some conformance test?
 > >
 > > <...>
 > >
 > > 	(void)copyout(msg->msg_ptr, msg_ptr, msg->msg_len);
 > > 
 > > The system call shouldn't fail silently, even if it means throwing the
 > > message away. The value from copyout should be returned.
 > 
 > POSIX defines, that if call fails - no message should be removed. In such
 > case, I think it is reasonable to validate the user-space pointer at start,
 > and ignore it later (as wrote in comment). Otherwise, if copyout() fails,
 > revert of message removal would be needed - that is problematic..

There are, unfortunately, two problems with what you've got: (1) you
haven't validated the whole block, only the first byte, so copyout can
still fail... and (2) even if you do validate the whole block in
advance, the copyout call can still fail if another thread has
rearranged the memory map in the meantime.

-- 
   - David A. Holland / dholland@eecs.harvard.edu