Re: [PATCH v2 3/6] tap: Guard IPv6 tail size subtraction against underflow

[Jon Maloy <jmaloy@redhat.com>]

On 2026-07-13 00:09, David Gibson wrote:
> On Fri, Jul 10, 2026 at 09:40:03AM -0400, Jon Maloy wrote:
>>
>>
>> On 2026-07-09 21:36, David Gibson wrote:
>>> On Thu, Jul 09, 2026 at 05:56:53PM -0400, Jon Maloy wrote:
>>>> In tap6_handler(), iov_tail_size(&data) - sizeof(*ip6h) computes
>>>> the expected payload length. IOV_PEEK_HEADER() guarantees at least
>>>> sizeof(*ip6h) bytes, but add an explicit check to guard the unsigned
>>>> subtraction. A too-small tail would indicate a malformed packet, so
>>>> skip it.
>>>>
>>>> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
>>>>
>>>> ---
>>>> v2: Use if-guard instead of assert(), to avoid runtime cost in
>>>>       per-packet path and to let the static checker follow the logic.
>>
>> I had the idea that the compiler may be smart enough to optimize it out,
>> since it is logically redundant, as you note.
> 
> I mean, maybe, although I think that's moderately unlikely.  But
> assert() is nothing but a macro that says essentially
> 	if (!condition) abort();
> So if the compiler can optimise away the if, it can optimise away the
> assert() as well.

Yes. But the analyzer will still mark it as an issue, which it won't if 
there is an if-clause.

/jon

> 
>>
>> /jon
>>
>>>
>>> Why would the if be cheaper than an assert()?  The IOV_PEEK_HEADER()
>>> already checks the length, so this check is definitely redundant - it
>>> exists only for the benefit of static checkers.
>>>
>>>> ---
>>>>    tap.c | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tap.c b/tap.c
>>>> index 6d93c7ce..6fd5f595 100644
>>>> --- a/tap.c
>>>> +++ b/tap.c
>>>> @@ -986,7 +986,10 @@ resume:
>>>>    		if (!ip6h)
>>>>    			continue;
>>>> -		check = iov_tail_size(&data) - sizeof(*ip6h);
>>>> +		check = iov_tail_size(&data);
>>>> +		if (check < sizeof(*ip6h))
>>>> +			continue;
>>>> +		check -= sizeof(*ip6h);
>>>>    		saddr = &ip6h->saddr;
>>>>    		daddr = &ip6h->daddr;
>>>> -- 
>>>> 2.52.0
>>>>
>>>
>>
>