[PATCH] packet: Fix off-by-one in packet_get_do() sanity checks

[PATCH] packet: Fix off-by-one in packet_get_do() sanity checks

[Stefano Brivio]

An n-sized pool, or a pool with n entries, doesn't include index n,
only up to n - 1.

I'm not entirely sure this sanity check actually covers any
practical case, but I spotted this while debugging a hang in
tap4_handler() (possibly due to malformed sequence entries from
qemu).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packet.c b/packet.c
index 3f82e84..d1ff998 100644
--- a/packet.c
+++ b/packet.c
@@ -87,7 +87,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
 void *packet_get_do(const struct pool *p, size_t index, size_t offset,
 		    size_t len, size_t *left, const char *func, int line)
 {
-	if (index > p->size || index > p->count) {
+	if (index >= p->size || index >= p->count) {
 		if (func) {
 			trace("packet %lu from pool size: %lu, count: %lu, "
 			      "%s:%i", index, p->size, p->count, func, line);
-- 
@@ -87,7 +87,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
 void *packet_get_do(const struct pool *p, size_t index, size_t offset,
 		    size_t len, size_t *left, const char *func, int line)
 {
-	if (index > p->size || index > p->count) {
+	if (index >= p->size || index >= p->count) {
 		if (func) {
 			trace("packet %lu from pool size: %lu, count: %lu, "
 			      "%s:%i", index, p->size, p->count, func, line);
-- 
2.35.1

Re: [PATCH] packet: Fix off-by-one in packet_get_do() sanity checks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Mon, Oct 10, 2022 at 09:53:11AM +0200, Stefano Brivio wrote:
> An n-sized pool, or a pool with n entries, doesn't include index n,
> only up to n - 1.
> 
> I'm not entirely sure this sanity check actually covers any
> practical case, but I spotted this while debugging a hang in
> tap4_handler() (possibly due to malformed sequence entries from
> qemu).
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  packet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/packet.c b/packet.c
> index 3f82e84..d1ff998 100644
> --- a/packet.c
> +++ b/packet.c
> @@ -87,7 +87,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
>  void *packet_get_do(const struct pool *p, size_t index, size_t offset,
>  		    size_t len, size_t *left, const char *func, int line)
>  {
> -	if (index > p->size || index > p->count) {
> +	if (index >= p->size || index >= p->count) {
>  		if (func) {
>  			trace("packet %lu from pool size: %lu, count: %lu, "
>  			      "%s:%i", index, p->size, p->count, func, line);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]