| Message ID | 20140606154231.GA7629@redhat.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 06/06/2014 07:42 PM, Dave Jones wrote: > > Hi all, > > While fuzzing with trinity inside a KVM tools guest running the latest -next > > kernel I've stumbled on the following spew: > > [ 269.531162] BUG: unable to handle kernel NULL pointer dereference at 000000000000021e > > [ 269.531217] IP: llc_ui_sendmsg (net/llc/af_llc.c:912) > 905 /* must bind connection to sap if user hasn't done it. */ > 906 if (sock_flag(sk, SOCK_ZAPPED)) { > 907 /* bind to sap with null dev, exclusive. */ > 908 rc = llc_ui_autobind(sock, addr); > 909 if (rc) > 910 goto release; > 911 } > 912 hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr); > looks like llc->dev was null, (understandable, given Trinity doesn't really know > how to set up llc beyond random socket()/setsockopt() calls). > llc_ui_autobind returns -ENODEV in that case, so it looks like the SOCK_ZAPPED test > was false. Something like the patch below maybe ? It feels ugly to be duplicating that > test there, but if this is agreed upon I'll resubmit this properly. > Dave > diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c > index 0080d2b0a8ae..9b192db9883b 100644 > --- a/net/llc/af_llc.c > +++ b/net/llc/af_llc.c > @@ -908,6 +908,11 @@ static int llc_ui_sendmsg(struct kiocb *iocb, struct socket *sock, > rc = llc_ui_autobind(sock, addr); > if (rc) > goto release; > + } else { > + if (!llc->dev) { } else if (!llc->dev) { > + rc = -ENODEV; > + goto release; > + } > } > hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr); > size = hdrlen + len; WBR, Sergei -- 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
On 06/06/2014 11:42 AM, Dave Jones wrote: > On Fri, Jun 06, 2014 at 11:08:33AM -0400, Sasha Levin wrote: > > Hi all, > > > > While fuzzing with trinity inside a KVM tools guest running the latest -next > > kernel I've stumbled on the following spew: > > > > [ 269.531162] BUG: unable to handle kernel NULL pointer dereference at 000000000000021e > > [ 269.531217] IP: llc_ui_sendmsg (net/llc/af_llc.c:912) > > 905 /* must bind connection to sap if user hasn't done it. */ > 906 if (sock_flag(sk, SOCK_ZAPPED)) { > 907 /* bind to sap with null dev, exclusive. */ > 908 rc = llc_ui_autobind(sock, addr); > 909 if (rc) > 910 goto release; > 911 } > 912 hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr); > > looks like llc->dev was null, (understandable, given Trinity doesn't really know > how to set up llc beyond random socket()/setsockopt() calls). > > llc_ui_autobind returns -ENODEV in that case, so it looks like the SOCK_ZAPPED test > was false. Something like the patch below maybe ? It feels ugly to be duplicating that > test there, but if this is agreed upon I'll resubmit this properly. I figured it's something more complicated than that since trinity stumbled on it only now and I don't see any code changes in that area. I'll test it with the patch. Thanks, Sasha -- 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
On 06/06/2014 01:53 PM, Sasha Levin wrote: > On 06/06/2014 11:42 AM, Dave Jones wrote: >> On Fri, Jun 06, 2014 at 11:08:33AM -0400, Sasha Levin wrote: >> > Hi all, >> > >> > While fuzzing with trinity inside a KVM tools guest running the latest -next >> > kernel I've stumbled on the following spew: >> > >> > [ 269.531162] BUG: unable to handle kernel NULL pointer dereference at 000000000000021e >> > [ 269.531217] IP: llc_ui_sendmsg (net/llc/af_llc.c:912) >> >> 905 /* must bind connection to sap if user hasn't done it. */ >> 906 if (sock_flag(sk, SOCK_ZAPPED)) { >> 907 /* bind to sap with null dev, exclusive. */ >> 908 rc = llc_ui_autobind(sock, addr); >> 909 if (rc) >> 910 goto release; >> 911 } >> 912 hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr); >> >> looks like llc->dev was null, (understandable, given Trinity doesn't really know >> how to set up llc beyond random socket()/setsockopt() calls). >> >> llc_ui_autobind returns -ENODEV in that case, so it looks like the SOCK_ZAPPED test >> was false. Something like the patch below maybe ? It feels ugly to be duplicating that >> test there, but if this is agreed upon I'll resubmit this properly. > > I figured it's something more complicated than that since trinity stumbled on it only > now and I don't see any code changes in that area. I'll test it with the patch. Same thing can happen in recvmsg as well, I wonder if there's a more generic fix rather than check for llc->dev everywhere. Thanks, Sasha -- 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/llc/af_llc.c b/net/llc/af_llc.c index 0080d2b0a8ae..9b192db9883b 100644 --- a/net/llc/af_llc.c +++ b/net/llc/af_llc.c @@ -908,6 +908,11 @@ static int llc_ui_sendmsg(struct kiocb *iocb, struct socket *sock, rc = llc_ui_autobind(sock, addr); if (rc) goto release; + } else { + if (!llc->dev) { + rc = -ENODEV; + goto release; + } } hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr); size = hdrlen + len;