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

[Jiri Slaby <jirislaby@kernel.org>]

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