[syslinux] [PATCH] core/diskboot.inc: fix handover info sanity checks

[Michal Soltys]

W dniu 21.02.2011 02:07, H. Peter Anvin pisze:
> On 02/20/2011 04:17 PM, Michal Soltys wrote:
>> Signed-off-by: Michal Soltys<soltys at ziu.info>
>> ---
>>   core/diskboot.inc |    6 +++---
>>   1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/core/diskboot.inc b/core/diskboot.inc
>> index 7c02066..3d48dfd 100644
>> --- a/core/diskboot.inc
>> +++ b/core/diskboot.inc
>> @@ -182,12 +182,12 @@ floppy:
>>   harddisk:
>>   		mov dx,[di-76-10]	; Original DS
>>   		mov si,[di-76-12]	; Original SI
>> -		shr si,4
>> -		jz .no_partition	; SI == 0 ->  assume no partition
>> +		shl dx,4
>>   		add dx,si
>> +		jz .no_partition	; DS:SI == 0 ->  assume no partition
>>   		cmp dx,1024		; DS:SI<  1K (inside the IVT)?
>>   		jb .no_partition
>> -		cmp dx,PartInfo>>  4	; DS:SI in overwritten memory?
>> +		cmp dx,PartInfo		; DS:SI in overwritten memory?
>>   		jae .no_partition
>>   		test byte [di-76],7Fh	; Sanity check: "active flag" should
>>   		jnz .no_partition	; be 00 or 80
> 
> No, this might overflow dx.
> 
> 	-hpa
> 

Indeed, but SI doesn't have to be divisable by 16.
E.g. typical handover address (if that's what DS:SI is here)
such as 0:7beh .

DX while it can overflow - considering addresses where handover is placed,
should be safe. With check against overflow, how about:

	test dh, 0xf0h
	jnz .no_partition	; overflow
	shl dx, 4
	add dx, si
	jc .no_partition	; overflow, DS:SI must be less than 64KiB
	cmp dx,1024		; DS:SI<  1K (inside the IVT)?
	jb .no_partition
	cmp dx, PartInfo-75
	jae .no_partition	; copied area (76 bytes) overlaps with PartInfo
	... 

This would guarantee:

DS:SI < 65536
DS:SI >= 1024
DS:SI + 75 < PartInfo

3rd implies 1st, but we have to check against overflow.