Bug 219000 - Integer underflow in efipart_realstrategy when I/O starts after end of disk
ClosedPublic
Actions

Authored by eric_metricspace.net on May 1 2017, 2:44 PM.

Details

Reviewers

allanjude
tsoome

Commits

rS320553: Integer underflow in efipart_realstrategy when I/O starts after end of disk

Summary

This fixes an integer underflow in efipart_realstrategy, which causes crashes when an I/O operation's start point is after the end of the disk.

This can happen when trying to detect filesystems on very small disks. This can occur if a BIOS freebsd-boot partition exists on a system when the EFI loader is being used.

Test Plan

This has been confirmed to fix the problem in the course of GELI debugging.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

eric_metricspace.net created this revision.May 1 2017, 2:44 PM

Herald added a subscriber: imp. · View Herald TranscriptMay 1 2017, 2:44 PM

tsoome added a subscriber: tsoome.May 1 2017, 2:52 PM

tsoome added inline comments.

sys/boot/efi/libefi/efipart.c
928 ↗	(On Diff #27888)	this will overflow as d_offset is uint64_t.
929 ↗	(On Diff #27888)	can this overflow?

eric_metricspace.net added inline comments.May 1 2017, 3:04 PM

sys/boot/efi/libefi/efipart.c
928 ↗	(On Diff #27888)	I thought size_t was a uint64_t

tsoome added inline comments.May 1 2017, 3:06 PM

sys/boot/efi/libefi/efipart.c
928 ↗	(On Diff #27888)	you also have to keep in mind that we will need the same code for UEFI32 - even as we do not built it at the moment:) Otherwise Allanjude will be real sad;)

Thanks!

sys/boot/efi/libefi/efipart.c
928–929 ↗	(On Diff #27888)	Style(9) puts these declarations at the top of the function.
934 ↗	(On Diff #27888)	Should we return an error? ENXIO / EINVAL?

tsoome added inline comments.May 1 2017, 3:24 PM

sys/boot/efi/libefi/efipart.c
934 ↗	(On Diff #27888)	The cross boundary reads in biosdisk.c are returning EIO. Also, it did occur me that it probably is good idea to try to keep the similar implementations as the basic logic should be the same anyhow. So I suggest to check biosdisk.c too (and maybe discover some hidden corner case there:D)

Rebased to HEAD

Addressed style comments

sys/boot/efi/libefi/efipart.c
929 ↗	(On Diff #27888)	Note: the original problem was an underflow, namely that (off / blkio->Media->BlockSize) was greater than (d_offset + disk_blocks), which would be negative, but wraps around to a really huge number because it's unsigned math. The only way to overflow here would be to have a partitions whose offsets exceed the capacity of size_t.