Bug 176748

Summary: [libi386] [patch] BTX Loader i386 incorrectly probes EDD, possibly resulting division by zero
Product: Base System Reporter: Arrigo Marchiori <ardovm>
Component: kernAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: imp, jhb
Priority: Normal Keywords: patch
Version: Unspecified   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff
none
ignore_zero_size.patch
none
Add consistency checks to EDD information none

Description Arrigo Marchiori 2013-03-08 13:40:01 UTC 
BTX probes for disk drives using interrupt 0x13.
The "EDD" information is parsed incorrectly: the loader may incorrectly assume that the EDD information is present, and therefore overwrite the number of sectors and their size with incorrect values.

The error is in file /usr/src/sys/boot/i386/libi386/biosdisk.c, inside function bd_int13probe().

The call to interrupt 0x13, AH=0x41 returns values in the bits of the CX register. The bit EDD_INTERFACE_EDD is not checked, while it should be.

The effect on my configuration was that bd_int13probe() incorrectly assumed that EDD information was present; it requested for it with int 0x13, AH=0x48 and overwrote the number and size of sectors with invalid values (zero, in my case). This resulted in a zero by zero division in file
/usr/src/sys/boot/i386/common/disk.c, function disk_open().

My setup had some buildworld/buildkernel tunings through src.conf(5). For instance, ZFS support was not compiled in. I believe such customization should not influence this particular problem.

Fix: The attached patch adds the check for the EDD_INTERFACE_EDD bit inside the data returned by the call to interrupt 0x13, AH=0x41.
It solves the problem on my hardware configuration.


Patch attached with submission follows:
How-To-Repeat: Find a system with a mass memory that does not provide EDD information.
This problem was detected on a AMD Geode GX CPU, using a USB drive for boot.
Please let me know if you need any more information about this.
Comment 1 Arrigo Marchiori 2014-01-14 16:21:13 UTC 
Hello,

this patch still applies to today's 9-STABLE.

Can I do anything to help integrating it?

Regards,
-- 
rigo

http://rigo.altervista.org
Comment 2 John Baldwin freebsd_committer freebsd_triage 2014-07-11 18:18:12 UTC 
Created attachment 144581 [details]
ignore_zero_size.patch

This patch is not correct.  The '48h' function is always required if EDD is present and is part of the 'Fixed Disk Access' subset as defined in EDD 3.0.

The part of the parameter block conditional on the 'EDD' subset returned by '48h' is the DPTE pointer (edd_params_seg and edd_params_off in 'struct edd_params').  The boot code does not use that.  In addition, while there is a flag in the 'flags' field (bit 2) that determines if the geometry returned in 'struct edd_params' is correct, that flag only covers the 'cylinders', 'heads', and 'sectors_per_track'.  The BIOS is always required to populate 'sectors' and 'sector_size' with valid values.

Instead, it seems your BIOS is just broken, but that can be worked around by ignoring sector counts of 0.
Comment 3 Arrigo Marchiori 2015-11-09 11:06:44 UTC 
Hello,

after some investigation, I could confirm that yes, the BIOS is broken.
However, the vendor of the embedded PC's I am using, is still applying the old and broken BIOS to recent units.

I only upgraded the FreeBSD sources last week, and the patch seems not to be effective any more, when booting from (recent) USB pen drives. For some weird reasons, the sector size of such USB sticks is returned as 12022 bytes or something like that, that is not correct.

Would it be advisable to add some more safety checks, such as:
  - sectors must be non-zero;
  - sector_size must be non-zero;
  - sector_size must be a multiple of 512?

Such a check enables my terminals to boot again.
Comment 4 Arrigo Marchiori 2015-11-09 11:16:56 UTC 
Created attachment 162919 [details]
Add consistency checks to EDD information

The EDD information is only used, if the sector count and sector size respect very simple requisites.
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2018-05-20 23:54:08 UTC 
For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"
Comment 6 Warner Losh freebsd_committer freebsd_triage 2019-01-07 18:57:06 UTC 
Different fallback code is in place now. Closing.