Re: [Qemu-devel] [PATCH] exec: check 'bounce.in_use' flag before using buffer

[P J P]

   Hello Peter,

+-- On Thu, 28 Jan 2016, Peter Maydell wrote --+
| This doesn't look right to me. The bounce buffer gets used
| if address_space_map() is called on something which isn't
| simple guest RAM. In this case address_space_map() will
| set bounce.in_use to true and return bounce.buffer as the
| mapped address. Then when the buffer is unmapped again,
| address_space_unmap() will finish using the bounce buffer
| and set bounce.in_use to false. You can only ever have one
| user of the bounce buffer at a time because address_space_map()
| will return NULL if it would need to use the bounce buffer
| but somebody else owns it.
| 
| So if we get into address_space_unmap() with a buffer
| value of bounce.buffer but bounce.in_use is false then
| something has already gone wrong. We need to figure out
| what that is.

  Yes, this is exactly same case, except that 'bounce.buffer' is NULL; It does 
not point to a valid address.

1. For first address_space_map() everything goes well and 'bounce.buffer' is 
   allocated.
2. For second address_space_map(), it returns NULL, because 'bounce.buffer' is 
   already in_use=true.

   ahci_port_write
    ahci_cond_start_engines
     ahci_map_fis_address
      map_page
       dma_memory_map
        address_space_map  <== returns NULL

3. For first address_space_unmap() everything goes well and 'bounce.buffer' is 
   set to NULL and 'bounce.in_use' is set to false.

4. For the second address_space_unmap(), the 'buffer' parameter is NULL 
   because second address_space_map() returned NULL.

   void address_space_unmap(..., void *buffer,)
   {
       if (buffer != bounce.buffer) {  <== both buffers are NULL
           ...
       }
       if (is_write) { <== is_write is true
           address_space_write(..., bounce.buffer=0x0, access_len);
            address_space_write_continue
             case 4:
                /* 32 bit write access */
                val = ldl_p(buf=0x0);
                       ldl_le_p
                        le_bswap
                         ldl_he_p
                          memcpy(&r, ptr=0x0, sizeof(r));  <== crash
       }
   }

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F