[ovs-dev] [PATCH v1] ofproto-dpif-upcall: fix for segmentation fault

[Justin Pettit]

> On Mar 5, 2018, at 4:09 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Mon, Mar 05, 2018 at 03:04:01PM -0800, Ashish Varma wrote:
>> Added check for NULL pointer on return from xlate_lookup_ofproto
>> function. Access to "ofproto" variable when NULL was causing segmentation
>> fault.
>> 
>> VMware-BZ: #2061914
>> Signed-off-by: Ashish Varma <ashishvarma.ovs at gmail.com>
>> ---
>> ofproto/ofproto-dpif-upcall.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>> 
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index 526be77..0079674 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -2129,6 +2129,11 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey,
>>         ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, &ofp_in_port);
>> 
>>         ofpbuf_clear(odp_actions);
>> +
>> +        if (!ofproto) {
>> +            goto exit;
>> +        }
>> +
>>         compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port,
>>                           ofp_in_port, odp_actions,
>>                           ofproto->up.slowpath_meter_id, &ofproto->uuid);
> 
> This bug got introduced in commit d39ec23de384 (ofproto-dpif: Don't
> slow-path controller actions.), which introduced the following change.
> The change seems to conscientiously decide that 'ofproto' could no
> longer be NULL, since it removes null tests.  Justin, do you think you
> just overlooked the possibility of null?  Ashish's commit will obviously
> fix the crash; do you think that it is the correct change?

I actually think it was an oversight of the reviewer.  (J'accuse.)

Yes, I agree that was in error.  Thanks for catching it, guys.

--Justin