On Sat, Apr 15, 2023 at 11:21:48PM +0800, James Raphael Tiovalen wrote: > This commit adds some `ovs_assert()` checks to the return values of > `dp_packet_data()` to ensure that they are not NULL and to prevent > null-pointer dereferences, which might lead to unwanted crashes. We use > assertions since it should be impossible for `dp_packet_data()` to > return NULL. > > Signed-off-by: James Raphael Tiovalen <[email protected]>
Hi James, thanks for persisting with this patchset. And thanks for splitting it up. That does make things a lot easier, at least for me. I think that the changes you introduce in this patch are both correct and nice. However, I have two questions: 1. I see that some users of dp_packet_data() are not modified by this patch. I did check all of them, but one example is in lib/bfd.c:bfd_process_packet Is that because they were not flagged by Coverity in your investigation? Or for some other reason (I often miss obvious points). 2. Did you consider moving the ovs_assert() into dp_packet_data(). Perhaps that is more risky because perahps sometimes it can legitimately be NULL. But, OTOH, if it is correct for dp_packet_data() to ever return NULL then it would nicely cover all cases. Unless I am missing something. Which often happens... _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
