On 6/24/22 16:00, Dumitru Ceara wrote:
> On 6/24/22 14:54, Ilya Maximets wrote:
>> ofpbuf_reserve() can be called with a zero size for a buffer with
>> an unallocated data.  It's a valid case, but we should not allow
>> evaluation of 'NULL + 0'.
>>
>>  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/ofpbuf.c:469:30 
>> in
>>  lib/ofpbuf.c:469:30: runtime error: applying zero offset to null pointer
>>     0 0xb2f890 in ofpbuf_reserve lib/ofpbuf.c:469:30
>>     1 0xb2f9bc in ofpbuf_new_with_headroom lib/ofpbuf.c:179:5
>>     2 0xb2f9bc in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:228:24
>>     3 0xb2f9bc in ofpbuf_clone_with_headroom lib/ofpbuf.c:199:18
>>     4 0xb2f8ea in ofpbuf_clone lib/ofpbuf.c:189:12
>>     5 0x6b3c57 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1712:5
>>     6 0x6c4315 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1738:5
>>     7 0x6beed6 in ukey_create_from_upcall 
>> ofproto/ofproto-dpif-upcall.c:1793:12
>>     8 0x6beed6 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1284:24
>>     9 0x6beed6 in process_upcall ofproto/ofproto-dpif-upcall.c:1456:9
>>     10 0x6bafb6 in recv_upcalls ofproto/ofproto-dpif-upcall.c:875:17
>>     11 0x6b70fa in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:792:13
>>     12 0xb4d5fa in ovsthread_wrapper lib/ovs-thread.c:422:12
>>     13 0x7fe6922081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
>>     14 0x7fe690e39dd2 in clone (/lib64/libc.so.6+0x39dd2)
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>> ---
>>  lib/ofpbuf.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
>> index 79ced46d7..679f3ba3e 100644
>> --- a/lib/ofpbuf.c
>> +++ b/lib/ofpbuf.c
>> @@ -464,6 +464,10 @@ ofpbuf_put_hex(struct ofpbuf *b, const char *s, size_t 
>> *n)
>>  void
>>  ofpbuf_reserve(struct ofpbuf *b, size_t size)
>>  {
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>>      ovs_assert(!b->size);
> 
> Just to be extra sure, I think I'd move the ovs_assert(!b->size) before
> your new check.
> 
> Anyhow, it's ok without or it can be done at apply time:
> 
> Acked-by: Dumitru Ceara <[email protected]>

Thanks, Aaron and Dumitru!  I moved the assert and applied the patch.
Also backported down to 2.13.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to