Re: [PATCH 1/6] tools/libxs: Fix length check in xs_talkv()

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2024-07-18 12:48, Andrew Cooper wrote:
> If the sum of iov element lengths overflows, the XENSTORE_PAYLOAD_MAX can
> pass, after which we'll write 4G of data with a good-looking length field, and
> the remainder of the payload will be interpreted as subsequent commands.
>
> Check each iov element length for XENSTORE_PAYLOAD_MAX before accmulating it.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> ---
>   tools/libs/store/xs.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
> index ec77379ab9bd..81a790cfe60f 100644
> --- a/tools/libs/store/xs.c
> +++ b/tools/libs/store/xs.c
> @@ -571,21 +571,24 @@ static void *xs_talkv(struct xs_handle *h, 
> xs_transaction_t t,
>         struct xsd_sockmsg msg;
>         void *ret = NULL;
>         int saved_errno;
> -       unsigned int i;
> +       unsigned int i, msg_len;
>         struct sigaction ignorepipe, oldact;
>   
>   	msg.tx_id = t;
>         msg.req_id = 0;
>         msg.type = type;
> -       msg.len = 0;
> -       for (i = 0; i < num_vecs; i++)
> -               msg.len += iovec[i].iov_len;
>   
> -	if (msg.len > XENSTORE_PAYLOAD_MAX) {
> -               errno = E2BIG;
> -               return 0;
> +       /* Calculate the payload length by summing iovec elements */
> +       for (i = 0, msg_len = 0; i < num_vecs; i++) {
> +               if ((iovec[i].iov_len > XENSTORE_PAYLOAD_MAX) ||
> +                   ((msg_len += iovec[i].iov_len) > XENSTORE_PAYLOAD_MAX)) {
> +                       errno = E2BIG;
> +                       return 0;
return NULL;

With that,
Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>