Re: [PATCH v3 12/13] migration/ram: Use offset_in_ramblock() in range checks

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* David Hildenbrand (david@redhat.com) wrote:
> We never read or write beyond the used_length of memory blocks when
> migrating. Make this clearer by using offset_in_ramblock() consistently.
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index ee5c3d5784..5cc9993899 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1309,8 +1309,8 @@ static bool find_dirty_block(RAMState *rs, PageSearchStatus *pss, bool *again)
>          *again = false;
>          return false;
>      }
> -    if ((((ram_addr_t)pss->page) << TARGET_PAGE_BITS)
> -        >= pss->block->used_length) {
> +    if (!offset_in_ramblock(pss->block,
> +                            ((ram_addr_t)pss->page) << TARGET_PAGE_BITS)) {
>          /* Didn't find anything in this RAM Block */
>          pss->page = 0;
>          pss->block = QLIST_NEXT_RCU(pss->block, next);
> @@ -1514,7 +1514,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len)
>          rs->last_req_rb = ramblock;
>      }
>      trace_ram_save_queue_pages(ramblock->idstr, start, len);
> -    if (start+len > ramblock->used_length) {
> +    if (!offset_in_ramblock(ramblock, start + len - 1)) {
>          error_report("%s request overrun start=" RAM_ADDR_FMT " len="
>                       RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
>                       __func__, start, len, ramblock->used_length);
> @@ -3325,8 +3325,8 @@ static void colo_flush_ram_cache(void)
>          while (block) {
>              offset = migration_bitmap_find_dirty(ram_state, block, offset);
>  
> -            if (((ram_addr_t)offset) << TARGET_PAGE_BITS
> -                >= block->used_length) {
> +            if (!offset_in_ramblock(block,
> +                                    ((ram_addr_t)offset) << TARGET_PAGE_BITS)) {
>                  offset = 0;
>                  block = QLIST_NEXT_RCU(block, next);
>              } else {
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK