[PATCH] tcp_vu: Fix assert/error-check order in tcp_vu_sock_recv()

[PATCH] tcp_vu: Fix assert/error-check order in tcp_vu_sock_recv()

[Jon Maloy]

iov_tail_clone() returns a ssize_t. However, the assert comparing
it against (ARRAY_SIZE(iov_msg) - j), which yields size_t, comes
before the negative-value check. Hence, when iov_tail_clone() fails
and returns -1, the signed-to-unsigned promotion turns -1 into
SIZE_MAX, and causes the assert to fire.

We fix this by moving the error check before the assert, while casting
the counter and the index inside the assert to size_t.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 tcp_vu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcp_vu.c b/tcp_vu.c
index 868a35b9..c052d8e2 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -291,9 +291,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
 
 		cnt = iov_tail_clone(&iov_msg[j], ARRAY_SIZE(iov_msg) - j,
 				     &data);
-		assert(cnt < ARRAY_SIZE(iov_msg) - j);
 		if (cnt < 0)
 			die("Missing entries in iov_msg");
+		assert((size_t)cnt < ARRAY_SIZE(iov_msg) - (size_t)j);
 
 		j += cnt;
 	}
-- 
2.52.0

Re: [PATCH] tcp_vu: Fix assert/error-check order in tcp_vu_sock_recv()

[Stefano Brivio]

[Cc: Johannes, Laurent]

Thanks for the patch and the explanation, but shouldn't this be:

On Mon,  9 Nov 2026 17:13:42 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

From: Johannes Bartunek <contact@jbartunek.de>

and:

> iov_tail_clone() returns a ssize_t. However, the assert comparing
> it against (ARRAY_SIZE(iov_msg) - j), which yields size_t, comes
> before the negative-value check. Hence, when iov_tail_clone() fails
> and returns -1, the signed-to-unsigned promotion turns -1 into
> SIZE_MAX, and causes the assert to fire.
> 
> We fix this by moving the error check before the assert, while casting
> the counter and the index inside the assert to size_t.

Reported-by: Johannes Bartunek <contact@jbartunek.de>
Suggested-by: Laurent Vivier <lvivier@redhat.com>

?

> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  tcp_vu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 868a35b9..c052d8e2 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -291,9 +291,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>  
>  		cnt = iov_tail_clone(&iov_msg[j], ARRAY_SIZE(iov_msg) - j,
>  				     &data);
> -		assert(cnt < ARRAY_SIZE(iov_msg) - j);
>  		if (cnt < 0)
>  			die("Missing entries in iov_msg");
> +		assert((size_t)cnt < ARRAY_SIZE(iov_msg) - (size_t)j);
>  
>  		j += cnt;
>  	}

-- 
Stefano

Re: [PATCH] tcp_vu: Fix assert/error-check order in tcp_vu_sock_recv()

[Laurent Vivier]

On 11/10/26 10:29, Stefano Brivio wrote:
> [Cc: Johannes, Laurent]
> 
> Thanks for the patch and the explanation, but shouldn't this be:
> 
> On Mon,  9 Nov 2026 17:13:42 -0500
> Jon Maloy <jmaloy@redhat.com> wrote:
> 
> From: Johannes Bartunek <contact@jbartunek.de>
> 
> and:
> 
>> iov_tail_clone() returns a ssize_t. However, the assert comparing
>> it against (ARRAY_SIZE(iov_msg) - j), which yields size_t, comes
>> before the negative-value check. Hence, when iov_tail_clone() fails
>> and returns -1, the signed-to-unsigned promotion turns -1 into
>> SIZE_MAX, and causes the assert to fire.
>>
>> We fix this by moving the error check before the assert, while casting
>> the counter and the index inside the assert to size_t.
> 
> Reported-by: Johannes Bartunek <contact@jbartunek.de>
> Suggested-by: Laurent Vivier <lvivier@redhat.com>

Not exactly...

> 
> ?
> 
>> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
>> ---
>>   tcp_vu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcp_vu.c b/tcp_vu.c
>> index 868a35b9..c052d8e2 100644
>> --- a/tcp_vu.c
>> +++ b/tcp_vu.c
>> @@ -291,9 +291,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>>   
>>   		cnt = iov_tail_clone(&iov_msg[j], ARRAY_SIZE(iov_msg) - j,
>>   				     &data);
>> -		assert(cnt < ARRAY_SIZE(iov_msg) - j);
>>   		if (cnt < 0)
>>   			die("Missing entries in iov_msg");
>> +		assert((size_t)cnt < ARRAY_SIZE(iov_msg) - (size_t)j);

The real fix is:

assert((size_t)cnt <= ARRAY_SIZE(iov_msg) - (size_t)j);

>>   
>>   		j += cnt;
>>   	}
> 

Thanks,
Laurent