| Message ID | 20171027154023.14307-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ofproto-dpif-upcall: Fix null pointer dereference on exit. | expand |
On Fri, 27 Oct 2017 08:40:23 -0700 Ben Pfaff <blp@ovn.org> wrote: > When revalidation occurs at the same time that a bridge is being removed > or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of > the ofproto. This caused a null pointer dereference if revalidation lost > the race. This commit fixes the problem. > > Reported-by: Jakub Sitnicki <jkbs@redhat.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- Tested-by: Jakub Sitnicki <jkbs@redhat.com> Thank you for the patch. I'm seeing less than before tests failing or hanging with it applied of on top of master. -Jakub
On Thu, 2 Nov 2017 18:53:02 +0100 Jakub Sitnicki <jkbs@redhat.com> wrote: > On Fri, 27 Oct 2017 08:40:23 -0700 > Ben Pfaff <blp@ovn.org> wrote: > > > When revalidation occurs at the same time that a bridge is being removed > > or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of > > the ofproto. This caused a null pointer dereference if revalidation lost > > the race. This commit fixes the problem. > > > > Reported-by: Jakub Sitnicki <jkbs@redhat.com> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > Tested-by: Jakub Sitnicki <jkbs@redhat.com> > > Thank you for the patch. I'm seeing less than before tests > failing or hanging with it applied of on top of master. Forgot to ask - any chance it could be also applied back to branch-2.7? Thanks, Jakub
On Thu, Nov 02, 2017 at 06:53:02PM +0100, Jakub Sitnicki wrote: > On Fri, 27 Oct 2017 08:40:23 -0700 > Ben Pfaff <blp@ovn.org> wrote: > > > When revalidation occurs at the same time that a bridge is being removed > > or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of > > the ofproto. This caused a null pointer dereference if revalidation lost > > the race. This commit fixes the problem. > > > > Reported-by: Jakub Sitnicki <jkbs@redhat.com> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > Tested-by: Jakub Sitnicki <jkbs@redhat.com> > > Thank you for the patch. I'm seeing less than before tests > failing or hanging with it applied of on top of master. Thanks. I applied this to master.
On Thu, Nov 02, 2017 at 06:58:39PM +0100, Jakub Sitnicki wrote: > On Thu, 2 Nov 2017 18:53:02 +0100 > Jakub Sitnicki <jkbs@redhat.com> wrote: > > > On Fri, 27 Oct 2017 08:40:23 -0700 > > Ben Pfaff <blp@ovn.org> wrote: > > > > > When revalidation occurs at the same time that a bridge is being removed > > > or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of > > > the ofproto. This caused a null pointer dereference if revalidation lost > > > the race. This commit fixes the problem. > > > > > > Reported-by: Jakub Sitnicki <jkbs@redhat.com> > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > --- > > > > Tested-by: Jakub Sitnicki <jkbs@redhat.com> > > > > Thank you for the patch. I'm seeing less than before tests > > failing or hanging with it applied of on top of master. > > Forgot to ask - any chance it could be also applied back to branch-2.7? I backported to branch-2.8, but branch-2.7 doesn't have the code that causes this problem.
On Thu, 2 Nov 2017 11:15:23 -0700 Ben Pfaff <blp@ovn.org> wrote: > > Forgot to ask - any chance it could be also applied back to branch-2.7? > > I backported to branch-2.8, but branch-2.7 doesn't have the code that > causes this problem. Thank you. I should have checked before asking. Will remember next time.
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 4a71bbe258df..e52871da2b81 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -2047,8 +2047,8 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey, if (xoutp->slow) { struct ofproto_dpif *ofproto; ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, NULL); - uint32_t smid = ofproto->up.slowpath_meter_id; - uint32_t cmid = ofproto->up.controller_meter_id; + uint32_t smid = ofproto ? ofproto->up.slowpath_meter_id : UINT32_MAX; + uint32_t cmid = ofproto ? ofproto->up.controller_meter_id : UINT32_MAX; ofpbuf_clear(odp_actions); compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port,
When revalidation occurs at the same time that a bridge is being removed or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of the ofproto. This caused a null pointer dereference if revalidation lost the race. This commit fixes the problem. Reported-by: Jakub Sitnicki <jkbs@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> --- ofproto/ofproto-dpif-upcall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)