[RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

Hi, Greg:

We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
was expected.

We tested it through the recvmsg01 testcase come from LTP testsuit. It set
msg->msg_namelen to -1 and the recvmsg syscall returned errno 14, which is
unexpected (errno 22 is expected):

recvmsg01    4  TFAIL  :  invalid socket length ; returned -1 (expected -1),
errno 14 (expected 22)

Linux mainline has no this bug for commit 08adb7dab fixes it accidentally.
However, it is too large and complex to be backported to LTS 3.10.

So, I made the following patch to fix the above problem for LTS 3.10.

Cheers,

Junling

============

Commit 281c9c36 (net: compat: Update get_compat_msghdr() to match
copy_msghdr_from_user() behaviour) made get_compat_msghdr() return
error if msg_sys->msg_namelen was negative, which changed the behaviors
of recvmsg and sendmsg syscall in a lib32 system:

Before commit 281c9c36, get_compat_msghdr() wouldn't fail and it would
return -EINVAL in move_addr_to_user() or somewhere if msg_sys->msg_namelen
was invalid and then syscall returned -EINVAL, which is correct.

And now, when msg_sys->msg_namelen is negative, get_compat_msghdr() will
fail and wants to return -EINVAL, however, the outer syscall will return
-EFAULT directly, which is unexpected.

This patch gets the return value of get_compat_msghdr() as well as
copy_msghdr_from_user(), then returns this expected value if
get_compat_msghdr() fails.

Fixes: 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour)
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
Signed-off-by: Hanbing Xu <xuhanbing@huawei.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Miller <davem@davemloft.net>
---
 net/socket.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index fc90b4f..53b6e41 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1988,14 +1988,12 @@ static int ___sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
 	int err, ctl_len, total_len;
 
 	err = -EFAULT;
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;
@@ -2200,14 +2198,12 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
 	struct sockaddr __user *uaddr;
 	int __user *uaddr_len;
 
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;
-- 
1.8.3.4

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[David Miller]

From: Junling Zheng <zhengjunling@huawei.com>
Date: Mon, 1 Jun 2015 09:28:00 +0000

> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
> was expected.
> 
> We tested it through the recvmsg01 testcase come from LTP testsuit. It set
> msg->msg_namelen to -1 and the recvmsg syscall returned errno 14, which is
> unexpected (errno 22 is expected):
> 
> recvmsg01    4  TFAIL  :  invalid socket length ; returned -1 (expected -1),
> errno 14 (expected 22)
> 
> Linux mainline has no this bug for commit 08adb7dab fixes it accidentally.
> However, it is too large and complex to be backported to LTS 3.10.
> 
> So, I made the following patch to fix the above problem for LTS 3.10.

Upstream the code doesn't look anything like this, so it's probably
just a missing bug fix for -stable.

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Greg KH]

On Mon, Jun 01, 2015 at 09:28:00AM +0000, Junling Zheng wrote:
> Hi, Greg:
> 
> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
> was expected.

That means I messed up and applied something I shouldn't have, right?

Can you use 'git bisect' to find the problem patch?  That's probably
easier here, and then I can either revert it, or fix up a broken
backport.

thanks,

greg k-h

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

On 2015/6/2 9:21, Greg KH wrote:
> On Mon, Jun 01, 2015 at 09:28:00AM +0000, Junling Zheng wrote:
>> Hi, Greg:
>>
>> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
>> was expected.
> 
> That means I messed up and applied something I shouldn't have, right?
> 
> Can you use 'git bisect' to find the problem patch?  That's probably
> easier here, and then I can either revert it, or fix up a broken
> backport.
> 

The following is the log of 'git bisect' between v3.10.72 and v3.10.73:

z00238152@Patch-Test:linux-3.10>0$ git bisect log
git bisect start
# good: [7f4e64246049cef5ae1eca37eec1701a9477799e] Linux 3.10.72
git bisect good 7f4e64246049cef5ae1eca37eec1701a9477799e
# bad: [73895725a9401bd3454757fcfa7d691270ac7498] Linux 3.10.73
git bisect bad 73895725a9401bd3454757fcfa7d691270ac7498
# bad: [d8bee0e3abef10e73a003fdc7ec04ba313334007] workqueue: fix hang involving racing cancel[_delayed]_work_sync()'s for PREEMPT_NONE
git bisect bad d8bee0e3abef10e73a003fdc7ec04ba313334007
# bad: [281c9c3601b69f1fbc492beb5976e1e2685854de] net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour
git bisect bad 281c9c3601b69f1fbc492beb5976e1e2685854de
# good: [e2f572a9be3f00bcf470a918d9e53353bea9ef4c] net: sysctl_net_core: check SNDBUF and RCVBUF for min length
git bisect good e2f572a9be3f00bcf470a918d9e53353bea9ef4c
# good: [c77b9a724d8597e1151b1fcf013560188088dc8c] caif: fix MSG_OOB test in caif_seqpkt_recvmsg()
git bisect good c77b9a724d8597e1151b1fcf013560188088dc8c
# good: [742c1c523e50723c7b7ff0af228c6711f7d747dd] Revert "net: cx82310_eth: use common match macro"
git bisect good 742c1c523e50723c7b7ff0af228c6711f7d747dd
# good: [175ff19c37149c352e3b983a8d78add034aef36f] tcp: fix tcp fin memory accounting
git bisect good 175ff19c37149c352e3b983a8d78add034aef36f
# first bad commit: [281c9c3601b69f1fbc492beb5976e1e2685854de] net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour
z00238152@Patch-Test:linux-3.10>0$

So, the problem commit is 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour), which fixes db31c55a6fb2
and brings the get_compat_msghdr() in line with copy_msghdr_from_user().

Cheers,

Junling

> thanks,
> 
> greg k-h
> 
> .
> 

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Greg KH]

On Tue, Jun 02, 2015 at 12:05:32PM +0800, Junling Zheng wrote:
> On 2015/6/2 9:21, Greg KH wrote:
> > On Mon, Jun 01, 2015 at 09:28:00AM +0000, Junling Zheng wrote:
> >> Hi, Greg:
> >>
> >> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
> >> was expected.
> > 
> > That means I messed up and applied something I shouldn't have, right?
> > 
> > Can you use 'git bisect' to find the problem patch?  That's probably
> > easier here, and then I can either revert it, or fix up a broken
> > backport.
> > 
> 
> The following is the log of 'git bisect' between v3.10.72 and v3.10.73:
> 
> z00238152@Patch-Test:linux-3.10>0$ git bisect log
> git bisect start
> # good: [7f4e64246049cef5ae1eca37eec1701a9477799e] Linux 3.10.72
> git bisect good 7f4e64246049cef5ae1eca37eec1701a9477799e
> # bad: [73895725a9401bd3454757fcfa7d691270ac7498] Linux 3.10.73
> git bisect bad 73895725a9401bd3454757fcfa7d691270ac7498
> # bad: [d8bee0e3abef10e73a003fdc7ec04ba313334007] workqueue: fix hang involving racing cancel[_delayed]_work_sync()'s for PREEMPT_NONE
> git bisect bad d8bee0e3abef10e73a003fdc7ec04ba313334007
> # bad: [281c9c3601b69f1fbc492beb5976e1e2685854de] net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour
> git bisect bad 281c9c3601b69f1fbc492beb5976e1e2685854de
> # good: [e2f572a9be3f00bcf470a918d9e53353bea9ef4c] net: sysctl_net_core: check SNDBUF and RCVBUF for min length
> git bisect good e2f572a9be3f00bcf470a918d9e53353bea9ef4c
> # good: [c77b9a724d8597e1151b1fcf013560188088dc8c] caif: fix MSG_OOB test in caif_seqpkt_recvmsg()
> git bisect good c77b9a724d8597e1151b1fcf013560188088dc8c
> # good: [742c1c523e50723c7b7ff0af228c6711f7d747dd] Revert "net: cx82310_eth: use common match macro"
> git bisect good 742c1c523e50723c7b7ff0af228c6711f7d747dd
> # good: [175ff19c37149c352e3b983a8d78add034aef36f] tcp: fix tcp fin memory accounting
> git bisect good 175ff19c37149c352e3b983a8d78add034aef36f
> # first bad commit: [281c9c3601b69f1fbc492beb5976e1e2685854de] net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour
> z00238152@Patch-Test:linux-3.10>0$
> 
> So, the problem commit is 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour), which fixes db31c55a6fb2
> and brings the get_compat_msghdr() in line with copy_msghdr_from_user().

Ok, but that patch itself isn't the issue, as it's "correct".  Something
else must be wrong here, did we backport db31c55a6fb2 incorrectly to
3.10-stable?  What is really causing this to be so different that your
proposed patch needs to be applied to solve it?

thanks,

greg k-h

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

On 2015/6/2 12:44, Greg KH wrote:
> On Tue, Jun 02, 2015 at 12:05:32PM +0800, Junling Zheng wrote:
>> On 2015/6/2 9:21, Greg KH wrote:
>>> On Mon, Jun 01, 2015 at 09:28:00AM +0000, Junling Zheng wrote:
>>>> Hi, Greg:
>>>>
>>>> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
>>>> was expected.
>>>
>>> That means I messed up and applied something I shouldn't have, right?
>>>
>>> Can you use 'git bisect' to find the problem patch?  That's probably
>>> easier here, and then I can either revert it, or fix up a broken
>>> backport.
>>>
>>
>> So, the problem commit is 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour), which fixes db31c55a6fb2
>> and brings the get_compat_msghdr() in line with copy_msghdr_from_user().
> 
> Ok, but that patch itself isn't the issue, as it's "correct".  Something

I don't think so. The commit 281c9c36 is indeed the issue. Its upstream commit 91edd09
has no problem in mainline, however it's not appropriate or complete for 3.10-stable.
It changes the return value of get_compat_msghdr(), however, the outer caller still
returns -EFAULT unconditionally, which goes against the intention of author.

So, I don't think 281c9c36 is an complete patch for 3.10-stable. We should also fix it
to return a correct value according to get_compat_msghdr() rather than returning -EFAULT
unconditionally :)

Cheers,

Junling

> else must be wrong here, did we backport db31c55a6fb2 incorrectly to
> 3.10-stable?  What is really causing this to be so different that your
> proposed patch needs to be applied to solve it?
> 
> thanks,
> 
> greg k-h
> 
> .
> 

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[David Miller]

From: Junling Zheng <zhengjunling@huawei.com>
Date: Tue, 2 Jun 2015 12:05:32 +0800

> So, the problem commit is 281c9c36 (net: compat: Update
> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
> with copy_msghdr_from_user().

Upstream this got fixed by:

08adb7dabd4874cc5666b4490653b26534702ce0

So the part that makes us not unconditionally return -EFAULT needs
to be backported, and that's probably equivalent to the patch
your proposed which therefore should be applied.

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Greg KH]

On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
> From: Junling Zheng <zhengjunling@huawei.com>
> Date: Tue, 2 Jun 2015 12:05:32 +0800
> 
> > So, the problem commit is 281c9c36 (net: compat: Update
> > get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
> > which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
> > with copy_msghdr_from_user().
> 
> Upstream this got fixed by:
> 
> 08adb7dabd4874cc5666b4490653b26534702ce0
> 
> So the part that makes us not unconditionally return -EFAULT needs
> to be backported, and that's probably equivalent to the patch
> your proposed which therefore should be applied.

Ok, thanks, now applied.

greg k-h

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

On 2015/6/2 14:27, Greg KH wrote:
> On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
>> From: Junling Zheng <zhengjunling@huawei.com>
>> Date: Tue, 2 Jun 2015 12:05:32 +0800
>>
>>> So, the problem commit is 281c9c36 (net: compat: Update
>>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
>>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
>>> with copy_msghdr_from_user().
>>
>> Upstream this got fixed by:
>>
>> 08adb7dabd4874cc5666b4490653b26534702ce0
>>
>> So the part that makes us not unconditionally return -EFAULT needs
>> to be backported, and that's probably equivalent to the patch
>> your proposed which therefore should be applied.
> 
> Ok, thanks, now applied.
> 

Maybe other stable version also needs this fix:)

> greg k-h
> 
> .
> 

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Willy Tarreau]

On Tue, Jun 02, 2015 at 02:43:54PM +0800, Junling Zheng wrote:
> On 2015/6/2 14:27, Greg KH wrote:
> > On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
> >> From: Junling Zheng <zhengjunling@huawei.com>
> >> Date: Tue, 2 Jun 2015 12:05:32 +0800
> >>
> >>> So, the problem commit is 281c9c36 (net: compat: Update
> >>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
> >>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
> >>> with copy_msghdr_from_user().
> >>
> >> Upstream this got fixed by:
> >>
> >> 08adb7dabd4874cc5666b4490653b26534702ce0
> >>
> >> So the part that makes us not unconditionally return -EFAULT needs
> >> to be backported, and that's probably equivalent to the patch
> >> your proposed which therefore should be applied.
> > 
> > Ok, thanks, now applied.
> > 
> 
> Maybe other stable version also needs this fix:)

Yes, from what I'm seeing, at least 3.2 and 2.6.32 need it as well.

Thanks,
Willy

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

On 2015/6/2 14:52, Willy Tarreau wrote:
> On Tue, Jun 02, 2015 at 02:43:54PM +0800, Junling Zheng wrote:
>> On 2015/6/2 14:27, Greg KH wrote:
>>> On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
>>>> From: Junling Zheng <zhengjunling@huawei.com>
>>>> Date: Tue, 2 Jun 2015 12:05:32 +0800
>>>>
>>>>> So, the problem commit is 281c9c36 (net: compat: Update
>>>>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
>>>>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
>>>>> with copy_msghdr_from_user().
>>>>
>>>> Upstream this got fixed by:
>>>>
>>>> 08adb7dabd4874cc5666b4490653b26534702ce0
>>>>
>>>> So the part that makes us not unconditionally return -EFAULT needs
>>>> to be backported, and that's probably equivalent to the patch
>>>> your proposed which therefore should be applied.
>>>
>>> Ok, thanks, now applied.
>>>
>>
>> Maybe other stable version also needs this fix:)
> 
> Yes, from what I'm seeing, at least 3.2 and 2.6.32 need it as well.
> 

Yeah, all other stable versions *except 3.19 and 4.0* may need this fix:)

> Thanks,
> Willy
> 
> 
> .
> 

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Luis Henriques]

On Tue, Jun 02, 2015 at 03:05:57PM +0800, Junling Zheng wrote:
> On 2015/6/2 14:52, Willy Tarreau wrote:
> > On Tue, Jun 02, 2015 at 02:43:54PM +0800, Junling Zheng wrote:
> >> On 2015/6/2 14:27, Greg KH wrote:
> >>> On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
> >>>> From: Junling Zheng <zhengjunling@huawei.com>
> >>>> Date: Tue, 2 Jun 2015 12:05:32 +0800
> >>>>
> >>>>> So, the problem commit is 281c9c36 (net: compat: Update
> >>>>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
> >>>>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
> >>>>> with copy_msghdr_from_user().
> >>>>
> >>>> Upstream this got fixed by:
> >>>>
> >>>> 08adb7dabd4874cc5666b4490653b26534702ce0
> >>>>
> >>>> So the part that makes us not unconditionally return -EFAULT needs
> >>>> to be backported, and that's probably equivalent to the patch
> >>>> your proposed which therefore should be applied.
> >>>
> >>> Ok, thanks, now applied.
> >>>
> >>
> >> Maybe other stable version also needs this fix:)
> > 
> > Yes, from what I'm seeing, at least 3.2 and 2.6.32 need it as well.
> > 
> 
> Yeah, all other stable versions *except 3.19 and 4.0* may need this fix:)

This seems to be correct.  I am queuing this fix for the 3.16 kernel
as well.  Thank you!

Cheers,
--
Luís

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Luis Henriques]

On Tue, Jun 02, 2015 at 03:05:57PM +0800, Junling Zheng wrote:
> On 2015/6/2 14:52, Willy Tarreau wrote:
> > On Tue, Jun 02, 2015 at 02:43:54PM +0800, Junling Zheng wrote:
> >> On 2015/6/2 14:27, Greg KH wrote:
> >>> On Mon, Jun 01, 2015 at 10:23:57PM -0700, David Miller wrote:
> >>>> From: Junling Zheng <zhengjunling@huawei.com>
> >>>> Date: Tue, 2 Jun 2015 12:05:32 +0800
> >>>>
> >>>>> So, the problem commit is 281c9c36 (net: compat: Update
> >>>>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
> >>>>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
> >>>>> with copy_msghdr_from_user().
> >>>>
> >>>> Upstream this got fixed by:
> >>>>
> >>>> 08adb7dabd4874cc5666b4490653b26534702ce0
> >>>>
> >>>> So the part that makes us not unconditionally return -EFAULT needs
> >>>> to be backported, and that's probably equivalent to the patch
> >>>> your proposed which therefore should be applied.
> >>>
> >>> Ok, thanks, now applied.
> >>>
> >>
> >> Maybe other stable version also needs this fix:)
> > 
> > Yes, from what I'm seeing, at least 3.2 and 2.6.32 need it as well.
> > 
> 
> Yeah, all other stable versions *except 3.19 and 4.0* may need this fix:)

This seems to be correct.  I am queuing this fix for the 3.16 kernel
as well.  Thank you!

Cheers,
--
Lu�s

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Junling Zheng]

On 2015/6/2 13:23, David Miller wrote:
> From: Junling Zheng <zhengjunling@huawei.com>
> Date: Tue, 2 Jun 2015 12:05:32 +0800
> 
>> So, the problem commit is 281c9c36 (net: compat: Update
>> get_compat_msghdr() to match copy_msghdr_from_user() behaviour),
>> which fixes db31c55a6fb2 and brings the get_compat_msghdr() in line
>> with copy_msghdr_from_user().
> 
> Upstream this got fixed by:
> 
> 08adb7dabd4874cc5666b4490653b26534702ce0
> 
> So the part that makes us not unconditionally return -EFAULT needs
> to be backported, and that's probably equivalent to the patch
> your proposed which therefore should be applied.
> 
> .
> 
Yes, we indeed missed some changes of 08adb7d, and these changes could
make commit 281c9c36 appropriate for stable version.
In addition, I think other stable version also needs this fix.

Cheers,

Junling

Patch "net: socket: Fix the wrong returns for recvmsg and sendmsg" has been added to the 3.10-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    net: socket: Fix the wrong returns for recvmsg and sendmsg

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     net-socket-fix-the-wrong-returns-for-recvmsg-and-sendmsg.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From zhengjunling@huawei.com  Tue Jun  2 15:26:33 2015
From: Junling Zheng <zhengjunling@huawei.com>
Date: Mon, 1 Jun 2015 09:28:00 +0000
Subject: net: socket: Fix the wrong returns for recvmsg and sendmsg
To: <gregkh@linuxfoundation.org>
Cc: <lizefan@huawei.com>, <viro@zeniv.linux.org.uk>, <davem@davemloft.net>, <xuhanbing@huawei.com>, <stable@vger.kernel.org>, <netdev@vger.kernel.org>
Message-ID: <1433150880-9976-1-git-send-email-zhengjunling@huawei.com>

From: Junling Zheng <zhengjunling@huawei.com>

Based on 08adb7dabd4874cc5666b4490653b26534702ce0 upstream.

We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
was expected.

We tested it through the recvmsg01 testcase come from LTP testsuit. It set
msg->msg_namelen to -1 and the recvmsg syscall returned errno 14, which is
unexpected (errno 22 is expected):

recvmsg01    4  TFAIL  :  invalid socket length ; returned -1 (expected -1),
errno 14 (expected 22)

Linux mainline has no this bug for commit 08adb7dab fixes it accidentally.
However, it is too large and complex to be backported to LTS 3.10.

Commit 281c9c36 (net: compat: Update get_compat_msghdr() to match
copy_msghdr_from_user() behaviour) made get_compat_msghdr() return
error if msg_sys->msg_namelen was negative, which changed the behaviors
of recvmsg and sendmsg syscall in a lib32 system:

Before commit 281c9c36, get_compat_msghdr() wouldn't fail and it would
return -EINVAL in move_addr_to_user() or somewhere if msg_sys->msg_namelen
was invalid and then syscall returned -EINVAL, which is correct.

And now, when msg_sys->msg_namelen is negative, get_compat_msghdr() will
fail and wants to return -EINVAL, however, the outer syscall will return
-EFAULT directly, which is unexpected.

This patch gets the return value of get_compat_msghdr() as well as
copy_msghdr_from_user(), then returns this expected value if
get_compat_msghdr() fails.

Fixes: 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour)
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
Signed-off-by: Hanbing Xu <xuhanbing@huawei.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/socket.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

--- a/net/socket.c
+++ b/net/socket.c
@@ -1988,14 +1988,12 @@ static int ___sys_sendmsg(struct socket
 	int err, ctl_len, total_len;
 
 	err = -EFAULT;
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;
@@ -2200,14 +2198,12 @@ static int ___sys_recvmsg(struct socket
 	struct sockaddr __user *uaddr;
 	int __user *uaddr_len;
 
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;


Patches currently in stable-queue which might be from zhengjunling@huawei.com are

queue-3.10/net-socket-fix-the-wrong-returns-for-recvmsg-and-sendmsg.patch

Patch "net: socket: Fix the wrong returns for recvmsg and sendmsg" has been added to the 3.14-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    net: socket: Fix the wrong returns for recvmsg and sendmsg

to the 3.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     net-socket-fix-the-wrong-returns-for-recvmsg-and-sendmsg.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From zhengjunling@huawei.com  Tue Jun  2 15:26:33 2015
From: Junling Zheng <zhengjunling@huawei.com>
Date: Mon, 1 Jun 2015 09:28:00 +0000
Subject: net: socket: Fix the wrong returns for recvmsg and sendmsg
To: <gregkh@linuxfoundation.org>
Cc: <lizefan@huawei.com>, <viro@zeniv.linux.org.uk>, <davem@davemloft.net>, <xuhanbing@huawei.com>, <stable@vger.kernel.org>, <netdev@vger.kernel.org>
Message-ID: <1433150880-9976-1-git-send-email-zhengjunling@huawei.com>

From: Junling Zheng <zhengjunling@huawei.com>

Based on 08adb7dabd4874cc5666b4490653b26534702ce0 upstream.

We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
was expected.

We tested it through the recvmsg01 testcase come from LTP testsuit. It set
msg->msg_namelen to -1 and the recvmsg syscall returned errno 14, which is
unexpected (errno 22 is expected):

recvmsg01    4  TFAIL  :  invalid socket length ; returned -1 (expected -1),
errno 14 (expected 22)

Linux mainline has no this bug for commit 08adb7dab fixes it accidentally.
However, it is too large and complex to be backported to LTS 3.10.

Commit 281c9c36 (net: compat: Update get_compat_msghdr() to match
copy_msghdr_from_user() behaviour) made get_compat_msghdr() return
error if msg_sys->msg_namelen was negative, which changed the behaviors
of recvmsg and sendmsg syscall in a lib32 system:

Before commit 281c9c36, get_compat_msghdr() wouldn't fail and it would
return -EINVAL in move_addr_to_user() or somewhere if msg_sys->msg_namelen
was invalid and then syscall returned -EINVAL, which is correct.

And now, when msg_sys->msg_namelen is negative, get_compat_msghdr() will
fail and wants to return -EINVAL, however, the outer syscall will return
-EFAULT directly, which is unexpected.

This patch gets the return value of get_compat_msghdr() as well as
copy_msghdr_from_user(), then returns this expected value if
get_compat_msghdr() fails.

Fixes: 281c9c36 (net: compat: Update get_compat_msghdr() to match copy_msghdr_from_user() behaviour)
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
Signed-off-by: Hanbing Xu <xuhanbing@huawei.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/socket.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

--- a/net/socket.c
+++ b/net/socket.c
@@ -2007,14 +2007,12 @@ static int ___sys_sendmsg(struct socket
 	int err, ctl_len, total_len;
 
 	err = -EFAULT;
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;
@@ -2219,14 +2217,12 @@ static int ___sys_recvmsg(struct socket
 	struct sockaddr __user *uaddr;
 	int __user *uaddr_len;
 
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
+	if (MSG_CMSG_COMPAT & flags)
+		err = get_compat_msghdr(msg_sys, msg_compat);
+	else
 		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	if (err)
+		return err;
 
 	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
 		err = -EMSGSIZE;


Patches currently in stable-queue which might be from zhengjunling@huawei.com are

queue-3.14/net-socket-fix-the-wrong-returns-for-recvmsg-and-sendmsg.patch

Re: [RFC] [PATCH] net: socket: Fix the wrong returns for recvmsg and sendmsg

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

On Mon, 2015-06-01 at 09:28 +0000, Junling Zheng wrote:
> Hi, Greg:
> 
> We found that after v3.10.73, recvmsg might return -EFAULT while -EINVAL
> was expected.
> 
> We tested it through the recvmsg01 testcase come from LTP testsuit. It set
> msg->msg_namelen to -1 and the recvmsg syscall returned errno 14, which is
> unexpected (errno 22 is expected):
> 
> recvmsg01    4  TFAIL  :  invalid socket length ; returned -1 (expected -1),
> errno 14 (expected 22)
> 
> Linux mainline has no this bug for commit 08adb7dab fixes it accidentally.
> However, it is too large and complex to be backported to LTS 3.10.
> 
> So, I made the following patch to fix the above problem for LTS 3.10.
[...]

It looks like the 3.2 branch has the same problem, so I've queued this
patch up there as well.  Thanks a lot.

Ben.

-- 
Ben Hutchings
One of the nice things about standards is that there are so many of them.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]