These issues have low impact because they require precise circumstances to trigger one of them. The disk must be > 2 TiB in size and either: - The primary GPT header is dammaged. - The freebsd-boot partiton is located farther than the first 2 TiB of the disc and one of its sectors takes place at a lba value that makes the higher 32 bits of this very value change. Errors and corrections folow: * Lines 117 - 118 main.3a: decl (%si) # 0x0(%si) = last sec (0-31) movw $2,%cx Should be: main.3a: subl $1, (%si) # 0x0(%si) = last sec (0-31) sbbl $0, 4(%si) movw $4,%cx -> Copies only two 16-bits words but it's a 64-bits value. Moreover, decrements this 64-bit value without care for a possible carry. * Line 131 movb $0x10,%cl repe cmpsb Should be: movw $0x10,%cx repe cmpsb -> It's CX the counter for repe not CL. It works as is but it's dangerous to keep that. * Lines 153 - 154 next_boot: incl (%si) # Next LBA adcl $0,4(%si) Should be: next_boot: addl $1, (%si) # Next LBA adcl $0,4(%si) -> inc instruction doesn't affect the carry flag. * Lines 174 - 175 incl GPT_ADDR+GPT_PART_LBA # Next sector adcl $0,GPT_ADDR+GPT_PART_LBA+4 Should be: addl $1, GPT_ADDR+GPT_PART_LBA. adcl $0,GPT_ADDR+GPT_PART_LBA+4 -> Same as before.
Created attachment 200996 [details] pmbr source code patched
For your information, I compiled the code corrected as indicated above and installed it on a 12.0-RELEASE. It works. However, I can't test the cases where the original pmbr would crash as I don't have a disk > 2 TiB.
https://reviews.freebsd.org/D31100
Thanks for patching pmbr. There is also something more on the same idea concerning gptboot and zfsgptboot (not a bug but additional features). You can find it here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235206
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=0ca9f1d4a3b772036309fb1c14262ec77c674c5d commit 0ca9f1d4a3b772036309fb1c14262ec77c674c5d Author: Emrion <kmachine@free.fr> AuthorDate: 2021-07-13 20:37:59 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2021-07-13 21:40:44 +0000 Fix pmbr issues > 2TB These issues have low impact because they require precise circumstances to trigger one of them. The disk must be > 2 TiB in size and either: - The primary GPT header is dammaged. - The freebsd-boot partiton is located farther than the first 2 TiB of the disc and one of its sectors takes place at a lba value that makes the higher 32 bits of this very value change. Errors and corrections folow: - decl and incl don't affect CF, so replace with subl/addl $1 - repe uses %cx, so move size to it with movw - moving a 64-bit value with %cx of 2 (should be 4) so addresses > 2TB will work. PR: 233180 Reviewed by: imp@ (applied patch using description in bug) Differential Revision: https://reviews.freebsd.org/D31100 stand/i386/pmbr/pmbr.S | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Test booted in qemu. It works the same as before for small drives. code looks good to my queue. Pushed to main branch queued to my MFC branches.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=b0b483b64be8533c8113500850ae8de2d127a98d commit b0b483b64be8533c8113500850ae8de2d127a98d Author: Emrion <kmachine@free.fr> AuthorDate: 2021-07-13 20:37:59 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2021-07-21 16:13:10 +0000 Fix pmbr issues > 2TB These issues have low impact because they require precise circumstances to trigger one of them. The disk must be > 2 TiB in size and either: - The primary GPT header is dammaged. - The freebsd-boot partiton is located farther than the first 2 TiB of the disc and one of its sectors takes place at a lba value that makes the higher 32 bits of this very value change. Errors and corrections folow: - decl and incl don't affect CF, so replace with subl/addl $1 - repe uses %cx, so move size to it with movw - moving a 64-bit value with %cx of 2 (should be 4) so addresses > 2TB will work. PR: 233180 Reviewed by: imp@ (applied patch using description in bug) Differential Revision: https://reviews.freebsd.org/D31100 (cherry picked from commit 0ca9f1d4a3b772036309fb1c14262ec77c674c5d) stand/i386/pmbr/pmbr.S | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3cd395b3b9d2ab70d589d27203904238f138442f commit 3cd395b3b9d2ab70d589d27203904238f138442f Author: Emrion <kmachine@free.fr> AuthorDate: 2021-07-13 20:37:59 +0000 Commit: Warner Losh <imp@FreeBSD.org> CommitDate: 2021-07-21 16:16:31 +0000 Fix pmbr issues > 2TB These issues have low impact because they require precise circumstances to trigger one of them. The disk must be > 2 TiB in size and either: - The primary GPT header is dammaged. - The freebsd-boot partiton is located farther than the first 2 TiB of the disc and one of its sectors takes place at a lba value that makes the higher 32 bits of this very value change. Errors and corrections folow: - decl and incl don't affect CF, so replace with subl/addl $1 - repe uses %cx, so move size to it with movw - moving a 64-bit value with %cx of 2 (should be 4) so addresses > 2TB will work. PR: 233180 Reviewed by: imp@ (applied patch using description in bug) Differential Revision: https://reviews.freebsd.org/D31100 (cherry picked from commit 0ca9f1d4a3b772036309fb1c14262ec77c674c5d) stand/i386/pmbr/pmbr.s | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)