On 230120 1447, Peter Maydell wrote:
> On Fri, 20 Jan 2023 at 14:42, Darren Kenny <darren.ke...@oracle.com> wrote:
> > Generally, this looks good, but I do have a comment below...
> >
> > On Thursday, 2023-01-19 at 02:00:02 -05, Alexander Bulekov wrote:
> > > Add a flag to the DeviceState, when a device is engaged in PIO/MMIO/DMA.
> > > This flag is set/checked prior to calling a device's MemoryRegion
> > > handlers, and set when device code initiates DMA. The purpose of this
> > > flag is to prevent two types of DMA-based reentrancy issues:
>
> > > diff --git a/softmmu/memory.c b/softmmu/memory.c
> > > index e05332d07f..90ffaaa4f5 100644
> > > --- a/softmmu/memory.c
> > > +++ b/softmmu/memory.c
> > > @@ -533,6 +533,7 @@ static MemTxResult access_with_adjusted_size(hwaddr
> > > addr,
> > > uint64_t access_mask;
> > > unsigned access_size;
> > > unsigned i;
> > > + DeviceState *dev = NULL;
> > > MemTxResult r = MEMTX_OK;
> > >
> > > if (!access_size_min) {
> > > @@ -542,6 +543,17 @@ static MemTxResult access_with_adjusted_size(hwaddr
> > > addr,
> > > access_size_max = 4;
> > > }
> > >
> > > + /* Do not allow more than one simultanous access to a device's IO
> > > Regions */
> > > + if (mr->owner &&
> > > + !mr->ram_device && !mr->ram && !mr->rom_device && !mr->readonly)
> > > {
> > > + dev = (DeviceState *) object_dynamic_cast(mr->owner,
> > > TYPE_DEVICE);
> >
> > I don't know how likely this is to happen, but according to:
> >
> > - https://qemu-project.gitlab.io/qemu/devel/qom.html#c.object_dynamic_cast
> >
> > it is possible for the object_dynamic_cast() function to return NULL,
> > so it might make sense to wrap the subsequent calls in a test of dev !=
> > NULL.
>
> Yes. This came up in a previous version of this:
> https://lore.kernel.org/qemu-devel/CAFEAcA8E4nDoAWcj-v-dED-0hDtXGjJNSp3A=kdgf8uocw0...@mail.gmail.com/
>
> It's generally a bug to call object_dynamic_cast() and then not check
> the return value.
>
Sorry I missed that - Will be fixed in V5.
-Alex
> thanks
> -- PMM
