Re: [Bug 1878057] Re: null-ptr dereference in megasas_command_complete

[Philippe Mathieu-Daudé]

Cc'ing Hannes who doesn't have a Launchpad account.

On 7/18/20 12:24 PM, Philippe Mathieu-Daudé wrote:
> Might be relevant:
> 
> commit 6df5718bd3ec56225c44cf96440c723c1b611b87
> Author: Hannes Reinecke <hare@suse.de>
> Date:   Wed Oct 29 13:00:15 2014 +0100
> 
>     megasas: Rework frame queueing algorithm
>     
>     Windows requires the frames to be unmapped, otherwise we run
>     into a race condition where the updated frame data is not
>     visible to the guest.
>     With that we can simplify the queue algorithm and use a bitmap
>     for tracking free frames.
> 
>  /*
>   * This absolutely needs to be locked if
>   * qemu ever goes multithreaded.
>   */
>  static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
>      hwaddr frame, uint64_t context, int count)
> 
> Using -trace scsi\* -trace megasas\*:
> 
> megasas_qf_enqueue frame 0x0 count 0 context 0x0 head 0x0 tail 0x0 busy 1
> megasas_handle_scsi LD SCSI dev 1/0/0 sdev 0x5555573f5560 xfer 0
> scsi_req_parsed target 0 lun 0 tag 0 command 53 dir 0 length 0
> scsi_req_parsed_lba target 0 lun 0 tag 0 command 53 lba 0
> scsi_req_alloc target 0 lun 0 tag 0
> scsi_disk_new_request Command: lun=0 tag=0x0 data= 0x35 0x00 0x00 0x00 0x00 
> 0x00 0x00 0x00 0x00 0x00
> megasas_scsi_nodata scmd 0: no data to be transferred
> megasas_mmio_invalid_writel addr 0x44: 0x3101
> megasas_mmio_invalid_writel addr 0x48: 0x44b0100
> megasas_mmio_invalid_writel addr 0x4c: 0x380100
> megasas_mmio_invalid_writel addr 0x50: 0x4b010000
> megasas_mmio_invalid_writel addr 0x54: 0x3f010004
> megasas_mmio_invalid_writel addr 0x58: 0x1000000
> megasas_mmio_invalid_writel addr 0x5c: 0x100044b
> megasas_mmio_invalid_writel addr 0x60: 0x46
> megasas_mmio_invalid_writel addr 0x64: 0x44b01
> megasas_mmio_invalid_writel addr 0x68: 0x4d01
> megasas_mmio_invalid_writel addr 0x6c: 0x44b0100
> megasas_mmio_invalid_writel addr 0x70: 0x540100
> megasas_mmio_invalid_writel addr 0x74: 0x4b010000
> megasas_mmio_invalid_writel addr 0x78: 0x5b010004
> megasas_mmio_invalid_writel addr 0x7c: 0x1000000
> megasas_mmio_invalid_writel addr 0x80: 0x100044b
> megasas_mmio_invalid_writel addr 0x84: 0x62
> megasas_mmio_invalid_writel addr 0x88: 0x44b01
> megasas_mmio_invalid_writel addr 0x8c: 0x6901
> megasas_mmio_invalid_writel addr 0x90: 0x44b0100
> megasas_mmio_invalid_writel addr 0x94: 0x700100
> megasas_mmio_invalid_writel addr 0x98: 0x4b010000
> megasas_mmio_invalid_writel addr 0x9c: 0x77010004
> megasas_mmio_writel reg MFI_ODCR0: 0x1000000
> megasas_mmio_invalid_writel addr 0xa4: 0x100044b
> megasas_mmio_invalid_writel addr 0xa8: 0x7e
> megasas_mmio_invalid_writel addr 0xac: 0x44b01
> megasas_mmio_invalid_writel addr 0xb0: 0x8501
> megasas_mmio_invalid_writel addr 0xb4: 0x44b0100
> megasas_mmio_invalid_writel addr 0xb8: 0x8c0100
> megasas_mmio_invalid_writel addr 0xbc: 0x4b010000
> megasas_mmio_writel reg MFI_IQPL: 0x4
> megasas_qf_new frame 0x1 addr 0x0
> megasas_enqueue_frame fr: 0x7fffa1e00000
> megasas_qf_enqueue frame 0x1 count 2 context 0x0 head 0x0 tail 0x0 busy 2
> megasas_init_firmware pa 0x0 
> megasas_init_queue queue at 0x0 len 0 head 0x0 tail 0x0 flags 0x0
> megasas_unmap_frame fr: 0x7fffa1e44b00
> megasas_unmap_frame fr: 0x7fffa1e00000
> megasas_qf_complete_noirq context 0x0 
> scsi_req_dequeue target 0 lun 0 tag 0
> scsi_aio_complete
> megasas_command_complete scmd 0: status 0x0, residual 0
> megasas_scsi_complete scmd 0: status 0x0, len 0/0
> 
> The frame is unmapped when the complete callback occurs.
> Then SIGSEGV in megasas_command_complete():
> 
> 1856 static void megasas_command_complete(SCSIRequest *req, uint32_t status,
> 1857                                      size_t resid)
> 1858 {
> 1859     MegasasCmd *cmd = req->hba_private;
> 1860     uint8_t cmd_status = MFI_STAT_OK;
> 1861 
> 1862     trace_megasas_command_complete(cmd->index, status, resid);
> 1863 
> 1864     if (req->io_canceled) {
> 1865         return;
> 1866     }
> 1867 
> 1868     if (cmd->dcmd_opcode != -1) {
> 1869         /*
> 1870          * Internal command complete
> 1871          */
> 1872         cmd_status = megasas_finish_internal_dcmd(cmd, req, resid);
> 1873         if (cmd_status == MFI_STAT_INVALID_STATUS) {
> 1874             return;
> 1875         }
> 1876     } else {
> 1877         req->status = status;
> 1878         trace_megasas_scsi_complete(cmd->index, req->status,
> 1879                                     cmd->iov_size, req->cmd.xfer);
> 1880         if (req->status != GOOD) {
> 1881             cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR;
> 1882         }
> 1883         if (req->status == CHECK_CONDITION) {
> 1884             megasas_copy_sense(cmd);
> 1885         }
> 1886 
> 1887         cmd->frame->header.scsi_status = req->status;
> 
>              ^^^^^^^^^^ This is NULL.
> 
> 1888     }
> 1889     cmd->frame->header.cmd_status = cmd_status;
> 1890     megasas_complete_command(cmd);
> 1891 }
> 
> ** Changed in: qemu
>        Status: New => Confirmed
>