Re: [Qemu-devel] [PATCH v2 2/2] memory: fix possible NULL pointer dereference

[Dima Stepanov <dimastep@yandex-team.ru>]

On Wed, Jul 11, 2018 at 03:09:13PM +0100, Peter Maydell wrote:
> On 11 July 2018 at 14:47, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Hi Dima,
> >
> > On 07/11/2018 05:34 AM, Dima Stepanov wrote:
> >> Gentle ping. CCing Paolo Bonzini.
> >>
> >> Regards, Dima.
> >>
> >> On Tue, Jun 19, 2018 at 05:12:16PM +0300, Dima Stepanov wrote:
> >>> Ping.
> >>>
> >>> Regards, Dima.
> >>>
> >>> On Wed, Jun 13, 2018 at 11:19:55AM +0300, Dima Stepanov wrote:
> >>>> In the memory_region_do_invalidate_mmio_ptr() routine the section
> >>>> variable is intialized by the memory_region_find() call. The section.mr
> >>>> field can be set to NULL.
> >>>>
> >>>> Add the check for NULL before trying to drop a section.
> >>>>
> >>>> Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>
> >>>> ---
> >>>>  memory.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/memory.c b/memory.c
> >>>> index 3212acc..bb45248 100644
> >>>> --- a/memory.c
> >>>> +++ b/memory.c
> >>>> @@ -2712,7 +2712,7 @@ static void memory_region_do_invalidate_mmio_ptr(CPUState *cpu,
> >>>>      /* Reset dirty so this doesn't happen later. */
> >>>>      cpu_physical_memory_test_and_clear_dirty(offset, size, 1);
> >>>>
> >>>> -    if (section.mr != mr) {
> >>>> +    if (section.mr && (section.mr != mr)) {
> >
> > section.mr can't be NULL here.
> >
> > You can give the static analyzer a hint using "assert(section.mr);"
> 
> Not in my view much point in messing with this code, though:
> (a) it's broken and unusable as it stands (race conditions)
> (b) it's obsoleted by the execute-from-mmio patchset
> http://patchwork.ozlabs.org/cover/942090/ and if/when that
> goes in it will all just get deleted.


Got it.

Thanks, Dima.

> 
> thanks
> -- PMM