Re: [v2] hw: misc: edu: fix 2 off-by-one errors

[Jiri Slaby]

On 17. 10. 22, 15:44, Alexander Bulekov wrote:
> On 221015 1710, Chris Friedt wrote:
> > From: Christopher Friedt <cfriedt@meta.com>
> >
> > In the case that size1 was zero, because of the explicit
> > 'end1 > addr' check, the range check would fail and the error
> > message would read as shown below. The correct comparison
> > is 'end1 >= addr' (or 'addr <= end1').
> >
> > EDU: DMA range 0x40000-0x3ffff out of bounds (0x40000-0x40fff)!
> >
> > At the opposite end, in the case that size1 was 4096, within()
> > would fail because of the non-inclusive check 'end1 < end2',
> > which should have been 'end1 <= end2'. The error message would
> > previously say
> >
> > EDU: DMA range 0x40000-0x40fff out of bounds (0x40000-0x40fff)!
> >
> > This change
> > 1. renames local variables to be more less ambiguous
> > 2. fixes the two off-by-one errors described above.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1254
> >
> > Signed-off-by: Christopher Friedt <cfriedt@meta.com>
>
> Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
>
> As a side-note, seems strange that edu_check_range will abort the entire
> VM if the check fails, rather than handling the error more elegantly.
> Maybe that's useful for students developing kernel drivers against the
> device.

Yes, that was exactly the intention. First, as a punishment as they do 
something really wrong. Second, so they notice -- writing something 
wrong to a register of a real HW often freezes a machine too. Especially 
when misprogramming a DMA controller.

OTOH, this sucks too. Ext4 (and other FS too) is fine, they don't lose 
data. However they need to freshly boot, repair FS and investigate/think 
a lot. This trial and run (and crash) takes several loops for some.

So I am for softening it a bit. But they still should be noticed in some 
obvious way.

thanks,
--
js
suse labs