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

[Jürgen Groß <jgross@xxxxxxxx>]

On 19.07.24 23:14, Jason Andryuk wrote:
> 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>
With the suggested change:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen