Message ID | 201001151024.59482.hartleys@visionengravers.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
From: H Hartley Sweeten <hartleys@visionengravers.com> Date: Fri, 15 Jan 2010 10:24:59 -0700 > ipv4/ip_sockglue.c: copy msg_control optval from user to kernel space > > In do_ip_getsockopt the char __user *optval is used directly in > IP_PKTOPTIONS for the msg.msg_control and not copied from > user to kernel address space. This produces a sparse warning: > > warning: incorrect type in assignment (different address spaces) > expected void *msg_control > got char [noderef] <asn:1>*optval > > Fix this by using copy _from_user to set msg.msg_control. > > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> This isn't right. We want the 'optval' pointer itself, not the data it points to, stored in msg.msg_control And 'msg_control' is, in this case a user pointer. It just isn't annotated (along with the rest of struct msghdr) with "__user" because we mix the usage of this object with kernel and user pointers. How did you test your change? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "H Hartley Sweeten" <hartleys@visionengravers.com> Date: Sat, 16 Jan 2010 01:22:21 -0500 > On Fri 1/15/2010 8:30 PM, David Miller wrote: >> How did you test your change? > > Hmm... I saw the sparse warning and tried this to fix it. The code compiled > fine and the warning was gone. When I booted the resulting kernel I didn't > see any issues. I must not have went down this code path in my testing. I'm going to ask you a second time. What was your test case? How did you test the change? I don't think you tested your change at all besides seeing that gcc would accept the code and sparse stopped spitting out a warning. And you're vagueness about your testing methodology will only work to confirm my suspicions. I find it unlikely, at best, for you to have tested that code path, as 'msg' is an uninitilized stack variable at this point in the code, so 'msg->msg_control' is going to be a garbage pointer, and therefore copying to it would result in a crash. I don't even think you read and understood the code you are editing. I suspect you just wanted to kill the sparse warning somehow, you found a way that made the compiler and sparse eat it, and you simply ran with it. And that really upsets me. Fixing sparse warnings should not be a mindless exercise. You should understand the code you are changing. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index cafad9b..8065456 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -1173,7 +1173,8 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname, if (sk->sk_type != SOCK_STREAM) return -ENOPROTOOPT; - msg.msg_control = optval; + if (copy_from_user(msg.msg_control, optval, len)) + return -EFAULT; msg.msg_controllen = len; msg.msg_flags = 0;
ipv4/ip_sockglue.c: copy msg_control optval from user to kernel space In do_ip_getsockopt the char __user *optval is used directly in IP_PKTOPTIONS for the msg.msg_control and not copied from user to kernel address space. This produces a sparse warning: warning: incorrect type in assignment (different address spaces) expected void *msg_control got char [noderef] <asn:1>*optval Fix this by using copy _from_user to set msg.msg_control. Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> Cc: David S. Miller <davem@davemloft.net> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi> Cc: James Morris <jmorris@namei.org> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> Cc: Patrick McHardy <kaber@trash.net> --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html