On 10/14/25 12:09 PM, Kevin Traynor via dev wrote:
> On 14/10/2025 10:15, Eelco Chaudron wrote:
>>
>>
>> On 14 Oct 2025, at 11:11, Kevin Traynor wrote:
>>
>>> On 13/10/2025 13:32, Eelco Chaudron via dev wrote:
>>>> This patch fixes a potential null pointer dereference reported
>>>> by Coverity if an null actions list is passed to nl_attr_get()
>>>> in odp_execute_sample().
>>>>
>>>> Fixes: 26c6b6cd2b2e ("dpif-netdev: Implement OVS_ACTION_ATTR_SAMPLE
>>>> action.")
>>>> Signed-off-by: Eelco Chaudron <[email protected]>
>>>> ---
>>>> v2: Actually check for the actions non-null
>>>> ---
>>>> lib/odp-execute.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
>>>> index 7f4e337f8..8943db2a5 100644
>>>> --- a/lib/odp-execute.c
>>>> +++ b/lib/odp-execute.c
>>>> @@ -739,6 +739,13 @@ odp_execute_sample(void *dp, struct dp_packet
>>>> *packet, bool steal,
>>>> }
>>>> }
>>>>
>>>> + if (!subactions || !nl_attr_get_size(subactions)) {
>>>> + if (steal) {
>>>> + dp_packet_delete(packet);
>>>
>>> Seems like it needs the coverage counter increased here as well
>>>
>>> COVERAGE_INC(datapath_drop_sample_error);
>>
>> Good catch, I can fold that in when applying. I’ll wait a bit longer
>> to see if any other comments come in.
FWIW, the original datapath_drop_sample_error makes no sense as is not
an error to drop a packet based on probability, it's just how the action
works. I'd say this counter shouldn't have existed. But it is suitable
for this new condition, which is actually an error if there are no
actions in the sample. At the same time if the action list exists, but
it is empty, I'm not sure that qualifies as error, it can be a normal
condition.
Best regards, Ilya Maximets.
>>
>
> ok, thanks. With coverage counter,
>
> Acked-by: Kevin Traynor <[email protected]>
>
>> Thanks,
>>
>> Eelco
>>
>>>> + }
>>>> + return;
>>>> + }
>>>> +
>>>> if (!steal) {
>>>> /* The 'subactions' may modify the packet, but the modification
>>>> * should not propagate beyond this sample action. Make a copy
>>
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
