Re: [PATCH for-5.2 0/3] hmp: Fix arg evaluation crash (regression)

[Dr. David Alan Gilbert]

* Kevin Wolf (kwolf@redhat.com) wrote:
> Am 13.11.2020 um 13:13 hat Dr. David Alan Gilbert geschrieben:
> > * Kevin Wolf (kwolf@redhat.com) wrote:
> > > When I restricted the section where the current monitor is set to only
> > > the command handler, I missed that monitor_parse_arguments() can use it
> > > indirectly, too, when evaluating register variables. These cases get
> > > NULL now and crash (easy to reproduce with "x $pc").
> > > 
> > > This series passes the right monitor object down instead of using
> > > monitor_cur(), which fixes the crash.
> > 
> > Why didn't the test-hmp.c find this?  It has a 'p $pc + 8'
> 
> Good question, a manual 'p $pc + 8' crashes for me on master.
> 
> Aha, it doesn't use a real HMP monitor, but QMP human-monitor-command.
> Then it would just get the wrong monitor (the QMP one instead of the
> temporary HMP monitor) and not NULL. The accessed CPU is even the same
> because neither QMP nor the temporary HMP monitor have a current CPU
> set, so even if the test case did check the result, it wouldn't catch
> this.
> 
> Only if the test case were using multiple CPUs and cpu-index had been
> set for human-monitor-command (to something other than the default), we
> would get a wrong result. But of course, it still wouldn't crash.

Ah, fair enough.

Dave

> Kevin
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK