hw/misc/imx7_ccm.c | 8 ++++++++ hw/misc/tz-ppc.c | 14 ++++++++++++++ hw/nvram/nrf51_nvm.c | 10 ++++++++++ hw/pci-host/designware.c | 19 +++++++++++++++++++ hw/pci-host/prep.c | 8 ++++++++ hw/ppc/prep_systemio.c | 8 ++++++++ hw/ppc/spapr_pci.c | 14 ++++++++++++-- hw/vfio/pci-quirks.c | 8 ++++++++ softmmu/memory.c | 10 +++++++++- 9 files changed, 96 insertions(+), 3 deletions(-)
From: Prasad J Pandit <pjp@fedoraproject.org> Hello, * This series asserts that MemoryRegionOps objects define read/write callback methods. Thus avoids potential NULL pointer dereference. ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 * Also adds various undefined MemoryRegionOps read/write functions to avoid potential assert failure. Thank you. -- Prasad J Pandit (9): hw/pci-host: add pci-intack write method pci-host: designware: add pcie-msi read method vfio: add quirk device write method prep: add ppc-parity write method nvram: add nrf51_soc flash read method spapr_pci: add spapr msi read method tz-ppc: add dummy read/write methods imx7-ccm: add digprog mmio write method memory: assert MemoryRegionOps callbacks are defined hw/misc/imx7_ccm.c | 8 ++++++++ hw/misc/tz-ppc.c | 14 ++++++++++++++ hw/nvram/nrf51_nvm.c | 10 ++++++++++ hw/pci-host/designware.c | 19 +++++++++++++++++++ hw/pci-host/prep.c | 8 ++++++++ hw/ppc/prep_systemio.c | 8 ++++++++ hw/ppc/spapr_pci.c | 14 ++++++++++++-- hw/vfio/pci-quirks.c | 8 ++++++++ softmmu/memory.c | 10 +++++++++- 9 files changed, 96 insertions(+), 3 deletions(-) -- 2.26.2
On Tue, Aug 11, 2020 at 05:11:24PM +0530, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Hello, > > * This series asserts that MemoryRegionOps objects define read/write > callback methods. Thus avoids potential NULL pointer dereference. > ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 > > * Also adds various undefined MemoryRegionOps read/write functions > to avoid potential assert failure. The overall idea seems fine. Looks like we could avoid a fair bit of boilerplate - and slightly reduce our binary size - by introducing a global unimplemented_write() function. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
+-- On Thu, 13 Aug 2020, David Gibson wrote --+ | The overall idea seems fine. Looks like we could avoid a fair bit of | boilerplate - and slightly reduce our binary size - by introducing a global | unimplemented_write() function. * You mean for after this assert(3) in memory_region_init_io change is merged? This series attempts to define all missing r/w calls. * There are also unassigned_mem_read/write functions, maybe those can be reused? Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
On 8/13/20 3:43 PM, P J P wrote: > +-- On Thu, 13 Aug 2020, David Gibson wrote --+ > | The overall idea seems fine. Looks like we could avoid a fair bit of > | boilerplate - and slightly reduce our binary size - by introducing a global > | unimplemented_write() function. IIRC in v2 or v3 it was mentioned each device has to be handled differently, as it behaves differently, it sits on a particular bus, and so on. So the preferred way is to have a device-specific handler. (This explains v4 and why these patches took so long). > > * You mean for after this assert(3) in memory_region_init_io change is merged? > This series attempts to define all missing r/w calls. > > * There are also unassigned_mem_read/write functions, maybe those can be > reused? > > > Thank you. > -- > Prasad J Pandit / Red Hat Product Security Team > 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D >
+-- On Tue, 11 Aug 2020, P J P wrote --+ | * This series asserts that MemoryRegionOps objects define read/write | callback methods. Thus avoids potential NULL pointer dereference. | ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 | | * Also adds various undefined MemoryRegionOps read/write functions | to avoid potential assert failure. | | Thank you. | -- | Prasad J Pandit (9): | hw/pci-host: add pci-intack write method | pci-host: designware: add pcie-msi read method | vfio: add quirk device write method | prep: add ppc-parity write method | nvram: add nrf51_soc flash read method | spapr_pci: add spapr msi read method | tz-ppc: add dummy read/write methods | imx7-ccm: add digprog mmio write method | memory: assert MemoryRegionOps callbacks are defined | | hw/misc/imx7_ccm.c | 8 ++++++++ | hw/misc/tz-ppc.c | 14 ++++++++++++++ | hw/nvram/nrf51_nvm.c | 10 ++++++++++ | hw/pci-host/designware.c | 19 +++++++++++++++++++ | hw/pci-host/prep.c | 8 ++++++++ | hw/ppc/prep_systemio.c | 8 ++++++++ | hw/ppc/spapr_pci.c | 14 ++++++++++++-- | hw/vfio/pci-quirks.c | 8 ++++++++ | softmmu/memory.c | 10 +++++++++- | 9 files changed, 96 insertions(+), 3 deletions(-) Ping...@Peter, @David, @Paolo, @Herve, @Alex, * This patch series is not pulled/merged yet, could you please help with it? (Pinging respective maintainers, as didn't get reply for last pings, not sure how to go about it.) Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
+-- On Tue, 15 Sep 2020, P J P wrote --+ | +-- On Tue, 11 Aug 2020, P J P wrote --+ | | * This series asserts that MemoryRegionOps objects define read/write | | callback methods. Thus avoids potential NULL pointer dereference. | | ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 | | | | * Also adds various undefined MemoryRegionOps read/write functions | | to avoid potential assert failure. | | | | Thank you. | | -- | | Prasad J Pandit (9): | | hw/pci-host: add pci-intack write method | | pci-host: designware: add pcie-msi read method | | vfio: add quirk device write method | | prep: add ppc-parity write method | | nvram: add nrf51_soc flash read method | | spapr_pci: add spapr msi read method | | tz-ppc: add dummy read/write methods | | imx7-ccm: add digprog mmio write method | | memory: assert MemoryRegionOps callbacks are defined | | | | hw/misc/imx7_ccm.c | 8 ++++++++ | | hw/misc/tz-ppc.c | 14 ++++++++++++++ | | hw/nvram/nrf51_nvm.c | 10 ++++++++++ | | hw/pci-host/designware.c | 19 +++++++++++++++++++ | | hw/pci-host/prep.c | 8 ++++++++ | | hw/ppc/prep_systemio.c | 8 ++++++++ | | hw/ppc/spapr_pci.c | 14 ++++++++++++-- | | hw/vfio/pci-quirks.c | 8 ++++++++ | | softmmu/memory.c | 10 +++++++++- | | 9 files changed, 96 insertions(+), 3 deletions(-) | | Ping...@Peter, @David, @Paolo, @Herve, @Alex, | | * This patch series is not pulled/merged yet, could you please help with it? Ping..! -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
On 25/11/20 14:50, P J P wrote: > +-- On Tue, 15 Sep 2020, P J P wrote --+ > | +-- On Tue, 11 Aug 2020, P J P wrote --+ > | | * This series asserts that MemoryRegionOps objects define read/write > | | callback methods. Thus avoids potential NULL pointer dereference. > | | ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 > | | > | | * Also adds various undefined MemoryRegionOps read/write functions > | | to avoid potential assert failure. > | | > | | Thank you. > | | -- > | | Prasad J Pandit (9): > | | hw/pci-host: add pci-intack write method > | | pci-host: designware: add pcie-msi read method > | | vfio: add quirk device write method > | | prep: add ppc-parity write method > | | nvram: add nrf51_soc flash read method > | | spapr_pci: add spapr msi read method > | | tz-ppc: add dummy read/write methods > | | imx7-ccm: add digprog mmio write method > | | memory: assert MemoryRegionOps callbacks are defined > | | > | | hw/misc/imx7_ccm.c | 8 ++++++++ > | | hw/misc/tz-ppc.c | 14 ++++++++++++++ > | | hw/nvram/nrf51_nvm.c | 10 ++++++++++ > | | hw/pci-host/designware.c | 19 +++++++++++++++++++ > | | hw/pci-host/prep.c | 8 ++++++++ > | | hw/ppc/prep_systemio.c | 8 ++++++++ > | | hw/ppc/spapr_pci.c | 14 ++++++++++++-- > | | hw/vfio/pci-quirks.c | 8 ++++++++ > | | softmmu/memory.c | 10 +++++++++- > | | 9 files changed, 96 insertions(+), 3 deletions(-) > | > | Ping...@Peter, @David, @Paolo, @Herve, @Alex, > | > | * This patch series is not pulled/merged yet, could you please help with it? > > > Ping..! > -- > Prasad J Pandit / Red Hat Product Security Team > 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D > Queued, thanks! Paolo
On 8/11/20 1:41 PM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Hello, > > * This series asserts that MemoryRegionOps objects define read/write > callback methods. Thus avoids potential NULL pointer dereference. > ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2 > > * Also adds various undefined MemoryRegionOps read/write functions > to avoid potential assert failure. What about read_with_attrs()/write_with_attrs()? It seems they are part of the same problem. > > Thank you. > -- > Prasad J Pandit (9): > hw/pci-host: add pci-intack write method > pci-host: designware: add pcie-msi read method > vfio: add quirk device write method > prep: add ppc-parity write method > nvram: add nrf51_soc flash read method > spapr_pci: add spapr msi read method > tz-ppc: add dummy read/write methods > imx7-ccm: add digprog mmio write method > memory: assert MemoryRegionOps callbacks are defined > > hw/misc/imx7_ccm.c | 8 ++++++++ > hw/misc/tz-ppc.c | 14 ++++++++++++++ > hw/nvram/nrf51_nvm.c | 10 ++++++++++ > hw/pci-host/designware.c | 19 +++++++++++++++++++ > hw/pci-host/prep.c | 8 ++++++++ > hw/ppc/prep_systemio.c | 8 ++++++++ > hw/ppc/spapr_pci.c | 14 ++++++++++++-- > hw/vfio/pci-quirks.c | 8 ++++++++ > softmmu/memory.c | 10 +++++++++- > 9 files changed, 96 insertions(+), 3 deletions(-) > > -- > 2.26.2 >
+-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
| On 8/11/20 1:41 PM, P J P wrote:
| > From: Prasad J Pandit <pjp@fedoraproject.org>
| > * This series asserts that MemoryRegionOps objects define read/write
| > callback methods. Thus avoids potential NULL pointer dereference.
| > ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
| >
| > * Also adds various undefined MemoryRegionOps read/write functions
| > to avoid potential assert failure.
|
| What about read_with_attrs()/write_with_attrs()? It seems they are part of
| the same problem.
* read/write_with_attrs function is called if read/write callback is not
defined
../softmmu/memory.c
if (mr->ops->write) {
... memory_region_write_accessor, mr,
} else {
... memory_region_write_with_attrs_accessor,
So, defining read/write methods may also address read/write_with_attrs
issue?
* $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
-> https://paste.centos.org/view/386c9597
It doesn't show an occurrence where one of the read/write_with_attrs is
missing.
* Nevertheless, if we need to define read/write_with_attrs routines, because
memory_region_init_io() would assert(3) for them
could that be a subsequent patch series please?
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
On 8/17/20 7:02 AM, P J P wrote:
> +-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | On 8/11/20 1:41 PM, P J P wrote:
> | > From: Prasad J Pandit <pjp@fedoraproject.org>
> | > * This series asserts that MemoryRegionOps objects define read/write
> | > callback methods. Thus avoids potential NULL pointer dereference.
> | > ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
> | >
> | > * Also adds various undefined MemoryRegionOps read/write functions
> | > to avoid potential assert failure.
> |
> | What about read_with_attrs()/write_with_attrs()? It seems they are part of
> | the same problem.
>
> * read/write_with_attrs function is called if read/write callback is not
> defined
>
> ../softmmu/memory.c
> if (mr->ops->write) {
> ... memory_region_write_accessor, mr,
> } else {
> ... memory_region_write_with_attrs_accessor,
>
> So, defining read/write methods may also address read/write_with_attrs
> issue?
>
> * $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
>
> -> https://paste.centos.org/view/386c9597
>
> It doesn't show an occurrence where one of the read/write_with_attrs is
> missing.
>
> * Nevertheless, if we need to define read/write_with_attrs routines, because
> memory_region_init_io() would assert(3) for them
>
> could that be a subsequent patch series please?
Yes no problem, I was just wondering and wasn't sure.
+-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
| On 8/17/20 7:02 AM, P J P wrote:
| > +-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
| > | What about read_with_attrs()/write_with_attrs()? It seems they are part
| > | of the same problem.
| >
| > * read/write_with_attrs function is called if read/write callback is not
| > defined
| >
| > ../softmmu/memory.c
| > if (mr->ops->write) {
| > ... memory_region_write_accessor, mr,
| > } else {
| > ... memory_region_write_with_attrs_accessor,
| >
| > So, defining read/write methods may also address read/write_with_attrs
| > issue?
| >
| > * $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
| >
| > -> https://paste.centos.org/view/386c9597
| >
| > It doesn't show an occurrence where one of the read/write_with_attrs is
| > missing.
| >
| > * Nevertheless, if we need to define read/write_with_attrs routines, because
| > memory_region_init_io() would assert(3) for them
| >
| > could that be a subsequent patch series please?
|
| Yes no problem, I was just wondering and wasn't sure.
@Peter, @Paolo: (To confirm)
Do we ping/reach out to respective maintainers for merging this series?
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
+-- On Mon, 17 Aug 2020, P J P wrote --+
| +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
| | On 8/17/20 7:02 AM, P J P wrote:
| | > +-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
| | > | What about read_with_attrs()/write_with_attrs()? It seems they are part
| | > | of the same problem.
| | >
| | > * read/write_with_attrs function is called if read/write callback is not
| | > defined
| | >
| | > ../softmmu/memory.c
| | > if (mr->ops->write) {
| | > ... memory_region_write_accessor, mr,
| | > } else {
| | > ... memory_region_write_with_attrs_accessor,
| | >
| | > So, defining read/write methods may also address read/write_with_attrs
| | > issue?
| | >
| | > * $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
| | >
| | > -> https://paste.centos.org/view/386c9597
| | >
| | > It doesn't show an occurrence where one of the read/write_with_attrs is
| | > missing.
| | >
| | > * Nevertheless, if we need to define read/write_with_attrs routines, because
| | > memory_region_init_io() would assert(3) for them
| | >
| | > could that be a subsequent patch series please?
| |
| | Yes no problem, I was just wondering and wasn't sure.
|
| @Peter, @Paolo: (To confirm)
| Do we ping/reach out to respective maintainers for merging this series?
Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
+-- On Thu, 27 Aug 2020, P J P wrote --+
| +-- On Mon, 17 Aug 2020, P J P wrote --+
| | +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
| | | On 8/17/20 7:02 AM, P J P wrote:
| | | > +-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
| | | > | What about read_with_attrs()/write_with_attrs()? It seems they are part
| | | > | of the same problem.
| | | >
| | | > * read/write_with_attrs function is called if read/write callback is not
| | | > defined
| | | >
| | | > ../softmmu/memory.c
| | | > if (mr->ops->write) {
| | | > ... memory_region_write_accessor, mr,
| | | > } else {
| | | > ... memory_region_write_with_attrs_accessor,
| | | >
| | | > So, defining read/write methods may also address read/write_with_attrs
| | | > issue?
| | | >
| | | > * $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
| | | >
| | | > -> https://paste.centos.org/view/386c9597
| | | >
| | | > It doesn't show an occurrence where one of the read/write_with_attrs is
| | | > missing.
| | | >
| | | > * Nevertheless, if we need to define read/write_with_attrs routines, because
| | | > memory_region_init_io() would assert(3) for them
| | | >
| | | > could that be a subsequent patch series please?
| | |
| | | Yes no problem, I was just wondering and wasn't sure.
| |
| | @Peter, @Paolo: (To confirm)
| | Do we ping/reach out to respective maintainers for merging this series?
|
Ping..!
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
© 2016 - 2024 Red Hat, Inc.