Re: 2.2.x BUG & PATCH: recvmsg() does not check msg_controllen correctly

[Philippe Troin <phil@fifi.org>]

"David S. Miller" <davem@redhat.com> writes:

>    From: Philippe Troin <phil@fifi.org>
>    Date: 03 Nov 2000 16:17:53 -0800
> 
>    Mmmh, no, if fdmax <= 0 (which happens when msg_controllen <
>    sizeof(struct cmsghdr)), then alls fds are passed, eventually
>    clobbering past ((char*)(msg_control)+m_controllen).
> 
>    Run the little test case if you're not convinced...
>    I stand by my patch :-)
> 
> If fdmax <= 0, no iterations of the "for (i=0" loop will run.
> 'i' will therefore be left equal to zero.  Therefore the next
> bit of code writing in the SOL_SOCKET/SCM_RIGHTS/etc. values
> will not run.
> 
> Next comes the test I modified, which will set MSG_CTRUNC.
> 
> Next scm_destroy(scm) is called which frees the list (this has to get
> called and is why I say your patch wasn't correct).
> 
> So where in this code are all the fds passed to the user in this case?
> I don't care what it actually does, I want to be shown why because as
> far as I see it doesn't do what you say it does.

Well, you should have ran my little test case...
No really :-)

All your explanations make sense.
When I re-read the code in scm.c, I had trouble figuring out why it
did not work before my patch and why it worked after...

Here it is:
	int fdmax = (msg->msg_controllen - sizeof(struct cmsghdr))/sizeof(int);

But, msg->msg_controllen is of type __kernel_size_t, which is unsigned
int (on i386).
Which means that if msg_controllen < sizeof(struct cmsghdr), then
fdmax is somewhere around 0x40000000, courtesy of the int->unsigned
int C promotion...
Ooops...

Yes I agree, mixing signed and unsigned arithmetic is evil... Doesn't
gcc have a flag for unsafe signed/unsigned mixtures ?

Would you consider this patch (or a variant) for inclusion ?

Phil.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/