From afc1a6cc15e866dfc387745688f99528ca67ca7f Mon Sep 17 00:00:00 2001 From: Prabhjot Singh Sethi Date: Fri, 29 Apr 2016 08:40:56 -0700 Subject: [PATCH] Fix NULL ptr access while logging Issue: ------ Flow Add req was processed after the flow was replaced with later gen-id, causing the flow to raise an evict event while creating the ksync entry itself, where LogFlow is called with NULL KSyncEntry pointer which doesnot have NULL check safety Fix: ---- Add NULL check safe gaurd for KSyncEntry pointer while logging Closes-Bug: 1573917 Change-Id: I32eb720ee0ca335b8a75544896bc781fd663a135 --- src/vnsw/agent/pkt/flow_entry.cc | 8 ++++---- src/vnsw/agent/vrouter/ksync/ksync_flow_index_manager.cc | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/vnsw/agent/pkt/flow_entry.cc b/src/vnsw/agent/pkt/flow_entry.cc index 92dd4eab222..6948371edab 100644 --- a/src/vnsw/agent/pkt/flow_entry.cc +++ b/src/vnsw/agent/pkt/flow_entry.cc @@ -2389,8 +2389,8 @@ void FlowEntry::LogFlow(FlowEventLog::Event event, FlowTableKSyncEntry* ksync, << " flow->flow_handle = " << flow_handle_ << " flow->gen_id = " << (int)gen_id_ << " ksync = " << (void *)ksync - << " Ksync->hash_id = " << ksync->hash_id() - << " Ksync->gen_id = " << (int)ksync->gen_id() + << " Ksync->hash_id = " << ((ksync != NULL) ? ksync->hash_id() : -1) + << " Ksync->gen_id = " << ((ksync != NULL) ? (int)ksync->gen_id() : 0) << " new_flow_handle = " << flow_handle << " new_gen_id = " << (int)gen_id); @@ -2411,8 +2411,8 @@ void FlowEntry::LogFlow(FlowEventLog::Event event, FlowTableKSyncEntry* ksync, log->flow_handle_ = flow_handle_; log->flow_gen_id_ = gen_id_; log->ksync_entry_ = ksync; - log->hash_id_ = ksync->hash_id(); - log->gen_id_ = ksync->gen_id(); + log->hash_id_ = (ksync != NULL) ? ksync->hash_id() : -1; + log->gen_id_ = (ksync != NULL) ? ksync->gen_id() : 0; log->vrouter_flow_handle_ = flow_handle; log->vrouter_gen_id_ = gen_id; } diff --git a/src/vnsw/agent/vrouter/ksync/ksync_flow_index_manager.cc b/src/vnsw/agent/vrouter/ksync/ksync_flow_index_manager.cc index 7025cf3f664..ba2f0e6d54f 100644 --- a/src/vnsw/agent/vrouter/ksync/ksync_flow_index_manager.cc +++ b/src/vnsw/agent/vrouter/ksync/ksync_flow_index_manager.cc @@ -204,6 +204,9 @@ uint8_t KSyncFlowIndexManager::AcquireIndexUnLocked(uint32_t index, // evict current entry evict_gen_id = old->gen_id(); if (flow) { + // KSyncEntry can be NULL at this point since acquire + // index can be done before creating KSyncEntry + // LogFlow needs to handle NULL KSyncEntry pointer flow->LogFlow(FlowEventLog::FLOW_EVICT, flow->ksync_entry_, flow->flow_handle(), evict_gen_id); proto_->EvictFlowRequest(flow, flow->flow_handle(),