Re: [Qemu-devel] [PATCH] tcg: assert last byte is in guest space

["Rémi Denis-Courmont" <remi@remlab.net>]

Le perjantaina 8. helmikuuta 2019, 20.12.13 EET Richard Henderson a écrit :
> On 2/8/19 8:37 AM, Rémi Denis-Courmont wrote:
> > Rather than assert that the first byte of a checked range is within the
> > guest address space, assert that the last byte is. The assertion is
> > moved past the overflow check to ensure that the last byte is actually
> > the one with the highest address.
> > 
> > Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
> > ---
> > 
> >  accel/tcg/translate-all.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> What are you trying to fix here?

As far as I can tell, the following code assumes that the entire range of 
checked addresses falls within the guest address range. So it makes sense to 
fail the assertion if the any byte is out of range, rather than only the first 
one.

> You've moved the assert past some returns, so that some cases that should
> not be handled by this function no longer get checked.  As the comment
> says, the address should already have been filtered by h2g_valid.

I find it generally hard to argue what an assertion should or should not do 
insofar as an assertion failure is supposed to be impossible. I am not in a 
position to decide if the assertion should fail in error cases that would be 
handled properly with a safe failure anyway - namely 0 bytes ranges and 
wrapping-around ranges.

However, it seems odd to live some undefined cases not failing the assertion, 
and I would not dare to add an extra assertion, so I replaced the one.

-- 
Rémi Denis-Courmont
http://www.remlab.net/