Re: [PATCH v2 07/10] migration/rdma: Sanity check RDMA_CONTROL_REGISTER_REQUEST on buflen

Peter Xu <[email protected]> writes:

> RDMA header supports head.repeat on RDMA_CONTROL_REGISTER_REQUEST, which
> can include >1 memory registrations.  The current code did check over
> head.repeat to guard against RDMA_CONTROL_MAX_COMMANDS_PER_MESSAGE, however
> it didn't further check the buffer size (head.len) to make sure the
> received data is large enough to include the repeated entries.  Check it.
>
> This is almost only to harden this piece of code, in reality on source side
> QEMU never uses repeat>1..  However since it's a protocol, still keep it.
> Check the buffer size instead.
>
> In case it's not obvious to new RDMA readers: head.len should be the size
> RDMA has last received, as qemu_rdma_exchange_get_response() checked on it
> against byte_len (which was further fetched from ibv_wc.byte_len in
> qemu_rdma_poll()).
>
> Reported-by: Tristan (@TristanInSec)
> Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4028
> Reviewed-by: Jinpu Wang <[email protected]>
> Signed-off-by: Peter Xu <[email protected]>
> ---
>  migration/rdma.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 683c24ba4d..f7356e759f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3360,6 +3360,14 @@ int rdma_registration_handle(QEMUFile *f)
>              reg_resp.repeat = head.repeat;
>              registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
>  
> +            /* Making sure the register buffers to read are valid */
> +            if (head.len != head.repeat * sizeof(RDMARegister)) {
> +                error_report("%s: Invalid RDMA_CONTROL_REGISTER_REQUEST "
> +                             "(head.repeat=%"PRIu32", head.len=%"PRIu32")",
> +                             __func__, head.repeat, head.len);
> +                goto err;
> +            }
> +
>              for (int count = 0; count < head.repeat; count++) {
>                  uint64_t chunk;
>                  uint8_t *chunk_start, *chunk_end;

Reviewed-by: Fabiano Rosas <[email protected]>