On 2025-03-04 07:05, Stefano Brivio wrote:On Mon, 3 Mar 2025 20:29:11 -0500 Jon Maloy <jmaloy(a)redhat.com> wrote:[...]v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson v5: - Added corresponding patches for IPv6 v6: - Fixed some small nits after comments from D. Gibson. v7: - Added handling of all rejected ICMP messages - Returning correct user data amount if IPv6 as per RFC 4884. v8: - Added MTU to ICMPv4 ICMP_FRAG_NEEDED messages. - Added ASSERT() validation to message creation functions. v9: - Using real source address of ICMP to complement destination address for originial UDP message when needed. Jon Maloy (4): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap: break out building of udp header from tap_udp6_send function udp: create and send ICMPv6 to local peer when applicableI was about to apply those, then I realised that Coverity Scan isn't happy about a few things, listed below. I didn't check if those are false positives (I can have a look later or within a couple of days unless you get to it first). 1. --- /home/sbrivio/passt/udp.c:448:2: Type: Out-of-bounds access (ARRAY_VS_SINGLETON) /home/sbrivio/passt/udp.c:440:2: 1. path: Condition "!(dlen <= 8)", taking false branch. /home/sbrivio/passt/udp.c:444:2: 2. path: Condition "ee->ee_type == 3", taking true branch. /home/sbrivio/passt/udp.c:444:2: 3. path: Condition "ee->ee_code == 4", taking true branch. /home/sbrivio/passt/udp.c:448:2: 4. address_of: Taking address with "&msg.ip4h" yields a singleton pointer. /home/sbrivio/passt/udp.c:448:2: 5. callee_ptr_arith: Passing "&msg.ip4h" to function "tap_push_ip4h" which uses it as an array. This might corrupt or misinterpret adjacent memory locations. /home/sbrivio/passt/tap.c:162:2: 5.1. ptr_arith: Performing pointer arithmetic on "ip4h" in expression "ip4h + 1". ---3. --- /home/sbrivio/passt/udp.c:449:2: Type: Out-of-bounds access (ARRAY_VS_SINGLETON) /home/sbrivio/passt/udp.c:440:2: 1. path: Condition "!(dlen <= 8)", taking false branch. /home/sbrivio/passt/udp.c:444:2: 2. path: Condition "ee->ee_type == 3", taking true branch. /home/sbrivio/passt/udp.c:444:2: 3. path: Condition "ee->ee_code == 4", taking true branch. /home/sbrivio/passt/udp.c:449:2: 4. address_of: Taking address with "&msg.uh" yields a singleton pointer. /home/sbrivio/passt/udp.c:449:2: 5. callee_ptr_arith: Passing "&msg.uh" to function "tap_push_uh4" which uses it as an array. This might corrupt or misinterpret adjacent memory locations. /home/sbrivio/passt/tap.c:190:2: 5.1. ptr_arith: Performing pointer arithmetic on "uh" in expression "uh + 1". ---[...] I installed coverity and tried it, of course with the same result. These are clearly false positives, and the first one is already in the upstream code, not added by me. I can probably get rid of them with some pointer gymnastics, but is it really worth it? BTW, I discovered a bug in patch #2 which I just fixed. I will post a v10 of my patches shortly. ///jon