Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id <S131748AbQKDDxj>; Fri, 3 Nov 2000 22:53:39 -0500
Received: (majordomo@vger.kernel.org) by vger.kernel.org
	id <S132063AbQKDDx2>; Fri, 3 Nov 2000 22:53:28 -0500
Received: from tantale.fifi.org ([216.15.47.52]:18569 "EHLO tantale.fifi.org")
	by vger.kernel.org with ESMTP id <S131748AbQKDDxS>;
	Fri, 3 Nov 2000 22:53:18 -0500
To: "David S. Miller" <davem@redhat.com>
Cc: linux-kernel@vger.kernel.org, alan@lxorguk.ukuu.org.uk
Subject: Re: 2.2.x BUG & PATCH: recvmsg() does not check msg_controllen correctly
In-Reply-To: <87n1fgvl7a.fsf@tantale.fifi.org>
 <200011032218.OAA12790@pizda.ninka.net> <878zr0vbda.fsf@tantale.fifi.org>
 <200011040038.QAA13178@pizda.ninka.net>
MIME-Version: 1.0 (generated by SEMI 1.12.1 - "[JR] Nonoichi")
Content-Type: text/plain; charset=US-ASCII
From: Philippe Troin <phil@fifi.org>
Date: 03 Nov 2000 19:53:04 -0800
In-Reply-To: <200011040038.QAA13178@pizda.ninka.net> ("David S. Miller"'s message of "Fri, 3 Nov 2000 16:38:12 -0800")
Message-ID: <87u29oz93z.fsf@tantale.fifi.org>
Lines: 49
User-Agent: Semi-gnus/6.10.12 SEMI/1.12.1 ([JR] Nonoichi) FLIM/1.12.7
 (Y?zaki) Emacs/20.7 (i386-debian-linux-gnu) MULE/4.0
 (HANANOEN)
Sender: linux-kernel-owner@vger.kernel.org
X-Mailing-List: linux-kernel@vger.kernel.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/