* [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference [not found] <cover.1355494956.git.dborkman@redhat.com> @ 2012-12-14 14:27 ` Daniel Borkmann 2012-12-14 15:12 ` Vlad Yasevich 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2012-12-14 14:27 UTC (permalink / raw) To: David Miller; +Cc: netdev, Vlad Yasevich During debugging a sctp problem, I hit a kernel NULL pointer dereference in the jprobes callback jsctp_sf_eat_sack(). This small patch fixes the panic. Cc: Vlad Yasevich <vyasevich@gmail.com> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> --- net/sctp/probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sctp/probe.c b/net/sctp/probe.c index bc6cd75..0a4e9d6 100644 --- a/net/sctp/probe.c +++ b/net/sctp/probe.c @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep, sp = asoc->peer.primary_path; - if ((full || sp->cwnd != lcwnd) && + if (sp && (full || sp->cwnd != lcwnd) && (!port || asoc->peer.port == port || ep->base.bind_addr.port == port)) { lcwnd = sp->cwnd; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference 2012-12-14 14:27 ` [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference Daniel Borkmann @ 2012-12-14 15:12 ` Vlad Yasevich 2012-12-14 16:57 ` Daniel Borkmann 0 siblings, 1 reply; 5+ messages in thread From: Vlad Yasevich @ 2012-12-14 15:12 UTC (permalink / raw) To: Daniel Borkmann; +Cc: David Miller, netdev On 12/14/2012 09:27 AM, Daniel Borkmann wrote: > During debugging a sctp problem, I hit a kernel NULL pointer > dereference in the jprobes callback jsctp_sf_eat_sack(). This > small patch fixes the panic. > > Cc: Vlad Yasevich <vyasevich@gmail.com> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com> > --- > net/sctp/probe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sctp/probe.c b/net/sctp/probe.c > index bc6cd75..0a4e9d6 100644 > --- a/net/sctp/probe.c > +++ b/net/sctp/probe.c > @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep, > > sp = asoc->peer.primary_path; > > - if ((full || sp->cwnd != lcwnd) && > + if (sp && (full || sp->cwnd != lcwnd) && > (!port || asoc->peer.port == port || > ep->base.bind_addr.port == port)) { > lcwnd = sp->cwnd; > Sorry, but this patch isn't making much sense. @sp points to the primary path of the association and that can not be null if we receiving SACKs on this association. sctp_sf_eat_sack_6_2(), which we are probing, is only called while the association is valid and all the transports still exist. It is also called under lock, so the transports can not go away while processing of the SACK. So unless there is some kind of jprobe issue, the pointer you are looking at should always be valid. -vlad ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference 2012-12-14 15:12 ` Vlad Yasevich @ 2012-12-14 16:57 ` Daniel Borkmann 2012-12-14 21:49 ` Daniel Borkmann 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2012-12-14 16:57 UTC (permalink / raw) To: Vlad Yasevich; +Cc: David Miller, netdev On 12/14/2012 04:12 PM, Vlad Yasevich wrote: > On 12/14/2012 09:27 AM, Daniel Borkmann wrote: >> During debugging a sctp problem, I hit a kernel NULL pointer >> dereference in the jprobes callback jsctp_sf_eat_sack(). This >> small patch fixes the panic. >> >> Cc: Vlad Yasevich <vyasevich@gmail.com> >> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> >> --- >> net/sctp/probe.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/sctp/probe.c b/net/sctp/probe.c >> index bc6cd75..0a4e9d6 100644 >> --- a/net/sctp/probe.c >> +++ b/net/sctp/probe.c >> @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep, >> >> sp = asoc->peer.primary_path; >> >> - if ((full || sp->cwnd != lcwnd) && >> + if (sp && (full || sp->cwnd != lcwnd) && >> (!port || asoc->peer.port == port || >> ep->base.bind_addr.port == port)) { >> lcwnd = sp->cwnd; > > Sorry, but this patch isn't making much sense. @sp points to the primary path of the association and that can not be null if we receiving > SACKs on this association. > > sctp_sf_eat_sack_6_2(), which we are probing, is only called while the association is valid and all the transports still exist. It is also called under lock, so the transports can not go away while processing of the SACK. So unless there is some kind of jprobe issue, the pointer > you are looking at should always be valid. Okay, I'll dig a bit deeper into that over the weekend. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference 2012-12-14 16:57 ` Daniel Borkmann @ 2012-12-14 21:49 ` Daniel Borkmann 2012-12-15 19:58 ` Daniel Borkmann 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2012-12-14 21:49 UTC (permalink / raw) To: Vlad Yasevich; +Cc: David Miller, netdev On Fri, Dec 14, 2012 at 5:57 PM, Daniel Borkmann <dborkman@redhat.com> wrote: > On 12/14/2012 04:12 PM, Vlad Yasevich wrote: >> On 12/14/2012 09:27 AM, Daniel Borkmann wrote: >>> >>> During debugging a sctp problem, I hit a kernel NULL pointer >>> dereference in the jprobes callback jsctp_sf_eat_sack(). This >>> small patch fixes the panic. >>> >>> Cc: Vlad Yasevich <vyasevich@gmail.com> >>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> >>> --- >>> net/sctp/probe.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/net/sctp/probe.c b/net/sctp/probe.c >>> index bc6cd75..0a4e9d6 100644 >>> --- a/net/sctp/probe.c >>> +++ b/net/sctp/probe.c >>> @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct >>> sctp_endpoint *ep, >>> >>> sp = asoc->peer.primary_path; >>> >>> - if ((full || sp->cwnd != lcwnd) && >>> + if (sp && (full || sp->cwnd != lcwnd) && >>> (!port || asoc->peer.port == port || >>> ep->base.bind_addr.port == port)) { >>> lcwnd = sp->cwnd; >> >> Sorry, but this patch isn't making much sense. @sp points to the primary >> path of the association and that can not be null if we receiving >> SACKs on this association. >> >> sctp_sf_eat_sack_6_2(), which we are probing, is only called while the >> association is valid and all the transports still exist. It is also called >> under lock, so the transports can not go away while processing of the SACK. >> So unless there is some kind of jprobe issue, the pointer >> you are looking at should always be valid. > > Okay, I'll dig a bit deeper into that over the weekend. That's just for the record, I'll do further tests / debugging tomorrow ... I compiled the net kernel with kprobes selftest / sanity test (kernel/test_kprobes.c) and it passed: [ 2.978082] Kprobe smoke test started [ 3.034014] Kprobe smoke test passed successfully There, also jprobes are tested. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference 2012-12-14 21:49 ` Daniel Borkmann @ 2012-12-15 19:58 ` Daniel Borkmann 0 siblings, 0 replies; 5+ messages in thread From: Daniel Borkmann @ 2012-12-15 19:58 UTC (permalink / raw) To: Vlad Yasevich; +Cc: David Miller, netdev On Fri, Dec 14, 2012 at 10:49 PM, Daniel Borkmann <danborkmann@iogearbox.net> wrote: > On Fri, Dec 14, 2012 at 5:57 PM, Daniel Borkmann <dborkman@redhat.com> wrote: >> On 12/14/2012 04:12 PM, Vlad Yasevich wrote: >>> On 12/14/2012 09:27 AM, Daniel Borkmann wrote: >>>> >>>> During debugging a sctp problem, I hit a kernel NULL pointer >>>> dereference in the jprobes callback jsctp_sf_eat_sack(). This >>>> small patch fixes the panic. >>>> >>>> Cc: Vlad Yasevich <vyasevich@gmail.com> >>>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> >>>> --- >>>> net/sctp/probe.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/net/sctp/probe.c b/net/sctp/probe.c >>>> index bc6cd75..0a4e9d6 100644 >>>> --- a/net/sctp/probe.c >>>> +++ b/net/sctp/probe.c >>>> @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct >>>> sctp_endpoint *ep, >>>> >>>> sp = asoc->peer.primary_path; >>>> >>>> - if ((full || sp->cwnd != lcwnd) && >>>> + if (sp && (full || sp->cwnd != lcwnd) && >>>> (!port || asoc->peer.port == port || >>>> ep->base.bind_addr.port == port)) { >>>> lcwnd = sp->cwnd; >>> >>> Sorry, but this patch isn't making much sense. @sp points to the primary >>> path of the association and that can not be null if we receiving >>> SACKs on this association. >>> >>> sctp_sf_eat_sack_6_2(), which we are probing, is only called while the >>> association is valid and all the transports still exist. It is also called >>> under lock, so the transports can not go away while processing of the SACK. >>> So unless there is some kind of jprobe issue, the pointer >>> you are looking at should always be valid. >> >> Okay, I'll dig a bit deeper into that over the weekend. > > That's just for the record, I'll do further tests / debugging tomorrow ... > > I compiled the net kernel with kprobes selftest / sanity test > (kernel/test_kprobes.c) and it passed: > > [ 2.978082] Kprobe smoke test started > [ 3.034014] Kprobe smoke test passed successfully > > There, also jprobes are tested. Found the problem, will send a new patch for the net tree in a couple of minutes. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-15 19:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1355494956.git.dborkman@redhat.com>
2012-12-14 14:27 ` [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference Daniel Borkmann
2012-12-14 15:12 ` Vlad Yasevich
2012-12-14 16:57 ` Daniel Borkmann
2012-12-14 21:49 ` Daniel Borkmann
2012-12-15 19:58 ` Daniel Borkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).