[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pci: check bus pointer before dereference
From: |
P J P |
Subject: |
Re: [PATCH] pci: check bus pointer before dereference |
Date: |
Wed, 30 Sep 2020 10:32:42 +0530 (IST) |
[+Paolo, +Fam Zheng - for scsi]
+-- On Mon, 28 Sep 2020, P J P wrote --+
| +-- On Wed, 16 Sep 2020, Peter Maydell wrote --+
| | On Wed, 16 Sep 2020 at 07:28, P J P <ppandit@redhat.com> wrote:
| | > ->
https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Flsi_nullptr1
| | > ==1183858==Hint: address points to the zero page.
| | > #0 pci_change_irq_level hw/pci/pci.c:259
| | > #1 pci_irq_handler hw/pci/pci.c:1445
| | > #2 pci_set_irq hw/pci/pci.c:1463
| | > #3 lsi_set_irq hw/scsi/lsi53c895a.c:488
| | > #4 lsi_update_irq hw/scsi/lsi53c895a.c:523
| | > #5 lsi_script_scsi_interrupt hw/scsi/lsi53c895a.c:554
| | > #6 lsi_execute_script hw/scsi/lsi53c895a.c:1149
| | > #7 lsi_reg_writeb hw/scsi/lsi53c895a.c:1984
| | > #8 lsi_io_write hw/scsi/lsi53c895a.c:2146
| ...
| | Generally we don't bother to assert() that pointers that shouldn't be NULL
| | really are NULL immediately before dereferencing them, because the
| | dereference provides an equally easy-to-debug crash to the assert, and so
| | the assert doesn't provide anything extra. assert()ing that a pointer is
| | non-NULL is more useful if it is done in a place that identifies the
problem
| | at an earlier and easier-to-debug point in execution rather than at a later
| | point which is distantly removed from the place where the bogus pointer was
| | introduced.
|
| * The NULL dereference above occurs because the 'pci_dev->qdev->parent_bus'
| address gets overwritten (with 0x0) during scsi 'Memory Move' operation in
|
| ../hw/scsi/lsi53c895a.c
| #define LSI_BUF_SIZE 4096
|
| lsi_mmio_write
| lsi_reg_writeb
| lsi_execute_script
| static void lsi_memcpy(LSIState *s, ... int count=12MB)
| {
| int n;
| uint8_t buf[LSI_BUF_SIZE];
|
| while (count) {
| n = (count > LSI_BUF_SIZE) ? LSI_BUF_SIZE : count;
| lsi_mem_read(s, src, buf, n); <== read from DMA memory
| lsi_mem_write(s, dest, buf, n); <== write to I/O memory
| src += n;
| dest += n;
| count -= n;
| }
| }
| ->
https://www.manualslib.com/manual/1407578/Lsi-Lsi53c895a.html?page=254#manual
|
| * Above loop moves data between DMA memory to i/o address space.
|
| * Going through the manual above, it seems 'Memory Move' can move upto 16MB
of
| data between memory spaces.
|
| * I tried to see a suitable fix, but couldn't get one.
|
| - Limiting 'count' value does not seem right, as allowed value is upto 16MB.
|
| - Manual above talks about moving data via 'dma_buf'. But it doesn't seem
to
| be used here.
|
| * During above loop, 'dest' address moves past its 'MemoryRegion mr' and
| overwrites the adjacent 'mr' memory area, overwritting 'parent_bus' value.
|
| Any thoughts/hints please...?
@Paolo, @Fam...wdyt?
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
- Re: [PATCH] pci: check bus pointer before dereference, P J P, 2020/09/15
- Re: [PATCH] pci: check bus pointer before dereference, Li Qiang, 2020/09/15
- Re: [PATCH] pci: check bus pointer before dereference, Philippe Mathieu-Daudé, 2020/09/15
- Re: [PATCH] pci: check bus pointer before dereference, P J P, 2020/09/16
- Re: [PATCH] pci: check bus pointer before dereference, Peter Maydell, 2020/09/16
- Re: [PATCH] pci: check bus pointer before dereference, P J P, 2020/09/28
- Re: [PATCH] pci: check bus pointer before dereference,
P J P <=
- Re: [PATCH] pci: check bus pointer before dereference, Igor Mammedov, 2020/09/30
- Re: [PATCH] pci: check bus pointer before dereference, P J P, 2020/09/30