[2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[Wang Chen]

Check length of setsockopt's optval, which provided by user, before copy it
from user space.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
 net/ipv6/ipv6_sockglue.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index bf2a686..1ad0ac9 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -557,6 +557,9 @@ done:
 	{
 		struct ipv6_mreq mreq;
 
+		if (optlen != sizeof(struct ipv6_mreq))
+			goto e_inval;
+
 		retv = -EPROTO;
 		if (inet_sk(sk)->is_icsk)
 			break;
@@ -595,6 +598,9 @@ done:
 		struct group_req greq;
 		struct sockaddr_in6 *psin6;
 
+		if (optlen != sizeof(struct group_req))
+			goto e_inval;
+
 		retv = -EFAULT;
 		if (copy_from_user(&greq, optval, sizeof(struct group_req)))
 			break;
-- 
1.5.3.4

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[YOSHIFUJI Hideaki / 吉藤英明]

In article <47F0AA16.1070900@cn.fujitsu.com> (at Mon, 31 Mar 2008 17:08:38 +0800), Wang Chen <wangchen@cn.fujitsu.com> says:

> Check length of setsockopt's optval, which provided by user, before copy it
> from user space.

Please use "<" not "==".

--yoshfuji

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[Wang Chen]

YOSHIFUJI Hideaki / 吉藤英明 said the following on 2008-3-31 17:15:
> In article <47F0AA16.1070900@cn.fujitsu.com> (at Mon, 31 Mar 2008 17:08:38 +0800), Wang Chen <wangchen@cn.fujitsu.com> says:
> 
>> Check length of setsockopt's optval, which provided by user, before copy it
>> from user space.
> 
> Please use "<" not "==".
> 

There are two reasons that why I only check whether
len of optval is equal to that one in user space.
1. We currently copy the entire structure from user
   to kernel.
2. In other cases, such as IPV6_LEAVE/JOIN_ANYCAST,
   we check whether the len is equal to struct's len.

So, Yoshifuji-san, can you tell me more about why we
should use "<"? :)

--WCN

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[YOSHIFUJI Hideaki / 吉藤英明]

In article <47F0C101.9090302@cn.fujitsu.com> (at Mon, 31 Mar 2008 18:46:25 +0800), Wang Chen <wangchen@cn.fujitsu.com> says:

> YOSHIFUJI Hideaki / 吉藤英明 said the following on 2008-3-31 17:15:
> > In article <47F0AA16.1070900@cn.fujitsu.com> (at Mon, 31 Mar 2008 17:08:38 +0800), Wang Chen <wangchen@cn.fujitsu.com> says:
> > 
> >> Check length of setsockopt's optval, which provided by user, before copy it
> >> from user space.
> > 
> > Please use "<" not "==".
> > 
> 
> There are two reasons that why I only check whether
> len of optval is equal to that one in user space.
> 1. We currently copy the entire structure from user
>    to kernel.
> 2. In other cases, such as IPV6_LEAVE/JOIN_ANYCAST,
>    we check whether the len is equal to struct's len.
> 
> So, Yoshifuji-san, can you tell me more about why we
> should use "<"? :)

POSIX, as we have some comments in that file ;-).
If it is shorter than expected, -EINVAL, otherwise, no error.
We need to fix other sites as well.

--yoshfuji

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[Wang Chen]

YOSHIFUJI Hideaki / 吉藤英明 said the following on 2008-3-31 18:53:
>>>> Check length of setsockopt's optval, which provided by user, before copy it
>>>> from user space.
>>> Please use "<" not "==".
>>>
>> There are two reasons that why I only check whether
>> len of optval is equal to that one in user space.
>> 1. We currently copy the entire structure from user
>>    to kernel.
>> 2. In other cases, such as IPV6_LEAVE/JOIN_ANYCAST,
>>    we check whether the len is equal to struct's len.
>>
>> So, Yoshifuji-san, can you tell me more about why we
>> should use "<"? :)
> 
> POSIX, as we have some comments in that file ;-).
> If it is shorter than expected, -EINVAL, otherwise, no error.
> We need to fix other sites as well.
> 

ooh, I see.
I will take a business trip to Japan tomorrow and I will finish
this patch since I come back weekend.
Thank you for your explanation.

--WCN

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[Wang Chen]

YOSHIFUJI Hideaki said the following on 2008-3-31 18:53:
> POSIX, as we have some comments in that file ;-).
> If it is shorter than expected, -EINVAL, otherwise, no error.
> We need to fix other sites as well.
> 

Check length of setsockopt's optval, which provided by user, before copy it
from user space.
For POSIX compliant, return -EINVAL for setsockopt of short lengths.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
 net/ipv6/ipv6_sockglue.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index bf2a686..a5ac121 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -557,6 +557,9 @@ done:
 	{
 		struct ipv6_mreq mreq;
 
+		if (optlen < sizeof(struct ipv6_mreq))
+			goto e_inval;
+
 		retv = -EPROTO;
 		if (inet_sk(sk)->is_icsk)
 			break;
@@ -576,7 +579,7 @@ done:
 	{
 		struct ipv6_mreq mreq;
 
-		if (optlen != sizeof(struct ipv6_mreq))
+		if (optlen < sizeof(struct ipv6_mreq))
 			goto e_inval;
 
 		retv = -EFAULT;
@@ -595,6 +598,9 @@ done:
 		struct group_req greq;
 		struct sockaddr_in6 *psin6;
 
+		if (optlen < sizeof(struct group_req))
+			goto e_inval;
+
 		retv = -EFAULT;
 		if (copy_from_user(&greq, optval, sizeof(struct group_req)))
 			break;
@@ -619,7 +625,7 @@ done:
 		struct group_source_req greqs;
 		int omode, add;
 
-		if (optlen != sizeof(struct group_source_req))
+		if (optlen < sizeof(struct group_source_req))
 			goto e_inval;
 		if (copy_from_user(&greqs, optval, sizeof(greqs))) {
 			retv = -EFAULT;
-- 
1.5.4

Re: [2.6 PATCH] IPV6: Check length of optval provided by user in setsockopt()

[YOSHIFUJI Hideaki / 吉藤英明]

In article <47F97BEF.2070205@cn.fujitsu.com> (at Mon, 07 Apr 2008 09:42:07 +0800), Wang Chen <wangchen@cn.fujitsu.com> says:

> YOSHIFUJI Hideaki said the following on 2008-3-31 18:53:
> > POSIX, as we have some comments in that file ;-).
> > If it is shorter than expected, -EINVAL, otherwise, no error.
> > We need to fix other sites as well.
> > 
> 
> Check length of setsockopt's optval, which provided by user, before copy it
> from user space.
> For POSIX compliant, return -EINVAL for setsockopt of short lengths.
> 
> Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>

Applied, thanks.

--yoshfuji