On 22/07/2024 10:19 am, Jürgen Groß wrote:
> 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.coop...@citrix.com>
>>> ---
>>> CC: Anthony PERARD <anthony.per...@vates.tech>
>>> CC: Juergen Gross <jgr...@suse.com>
>>> ---
>>> 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.andr...@amd.com>
>
> With the suggested change:
>
> Reviewed-by: Juergen Gross <jgr...@suse.com>
Thankyou both. I'd not even spotted the mistake when just rearranging
the logic.
~Andrew
