[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 10/28] Sanity check RDMA remote data
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PULL 10/28] Sanity check RDMA remote data |
Date: |
Thu, 9 Jul 2015 15:41:57 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Paolo Bonzini (address@hidden) wrote:
>
>
> On 07/07/2015 15:08, Juan Quintela wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Perform some basic (but probably not complete) sanity checking on
> > requests from the RDMA source.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > Reviewed-by: Michael R. Hines <address@hidden>
> > Signed-off-by: Juan Quintela <address@hidden>
> > ---
> > migration/rdma.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index 73844a3..73a79be 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -2992,6 +2992,13 @@ static int qemu_rdma_registration_handle(QEMUFile
> > *f, void *opaque)
> > trace_qemu_rdma_registration_handle_compress(comp->length,
> > comp->block_idx,
> > comp->offset);
> > + if (comp->block_idx >= rdma->local_ram_blocks.nb_blocks) {
> > + error_report("rdma: 'compress' bad block index %u (vs %d)",
> > + (unsigned int)comp->block_idx,
> > + rdma->local_ram_blocks.nb_blocks);
> > + ret = -EIO;
> > + break;
>
> Did you want "goto out" here, and especially inside the for loop below:
Yes, it was nice of Coverity to spot that.
> > + }
> > block = &(rdma->local_ram_blocks.block[comp->block_idx]);
> >
> > host_addr = block->local_host_addr +
> > @@ -3080,8 +3087,23 @@ static int qemu_rdma_registration_handle(QEMUFile
> > *f, void *opaque)
> > trace_qemu_rdma_registration_handle_register_loop(count,
> > reg->current_index, reg->key.current_addr,
> > reg->chunks);
> >
> > + if (reg->current_index >=
> > rdma->local_ram_blocks.nb_blocks) {
> > + error_report("rdma: 'register' bad block index %u (vs
> > %d)",
> > + (unsigned int)reg->current_index,
> > + rdma->local_ram_blocks.nb_blocks);
> > + ret = -ENOENT;
> > + break;
>
> Here
>
> > + }
> > block =
> > &(rdma->local_ram_blocks.block[reg->current_index]);
> > if (block->is_ram_block) {
> > + if (block->offset > reg->key.current_addr) {
> > + error_report("rdma: bad register address for block
> > %s"
> > + " offset: %" PRIx64 " current_addr: %" PRIx64,
> > + block->block_name, block->offset,
> > + reg->key.current_addr);
> > + ret = -ERANGE;
> > + break;
>
> here
>
> > + }
> > host_addr = (block->local_host_addr +
> > (reg->key.current_addr - block->offset));
> > chunk = ram_chunk_index(block->local_host_addr,
> > @@ -3090,6 +3112,14 @@ static int qemu_rdma_registration_handle(QEMUFile
> > *f, void *opaque)
> > chunk = reg->key.chunk;
> > host_addr = block->local_host_addr +
> > (reg->key.chunk * (1UL << RDMA_REG_CHUNK_SHIFT));
> > + /* Check for particularly bad chunk value */
> > + if (host_addr < (void *)block->local_host_addr) {
> > + error_report("rdma: bad chunk for block %s"
> > + " chunk: %" PRIx64,
> > + block->block_name, reg->key.chunk);
> > + ret = -ERANGE;
> > + break;
>
> and here the "break" takes you directly to
>
> ret = qemu_rdma_post_send_control(rdma,
> (uint8_t *) results, ®_resp);
>
> where ret is overwritten (spotted by Coverity).
Yes, it needs the goto's back.
Dave
>
> Paolo
>
>
> Paolo
>
> > + }
> > }
> > chunk_start = ram_chunk_start(block, chunk);
> > chunk_end = ram_chunk_end(block, chunk + reg->chunks);
> >
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PULL 02/28] Only try and read a VMDescription if it should be there, (continued)
- [Qemu-devel] [PULL 02/28] Only try and read a VMDescription if it should be there, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 05/28] Translate offsets to destination address space, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 03/28] rdma typos, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 04/28] Store block name in local blocks structure, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 07/28] Allow rdma_delete_block to work without the hash, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 06/28] Rework ram_control_load_hook to hook during block load, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 08/28] Rework ram block hash, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 11/28] Fail more cleanly in mismatched RAM cases, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 10/28] Sanity check RDMA remote data, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 12/28] Fix older machine type compatibility on power with section footers, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 09/28] Sort destination RAMBlocks to be the same as the source, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 16/28] global_state: Make section optional, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 13/28] runstate: Add runstate store, Juan Quintela, 2015/07/07
- [Qemu-devel] [PULL 15/28] migration: create new section to store global state, Juan Quintela, 2015/07/07