On Thu, 3 Dec 2020 13:45:48 -0500 Rich Felker <dal...@libc.org> wrote:
> Commit a5f69f396713ab8ac1e57458cbb9af552d2c1659 rearranged bsd.c's > bsd_probe function in a way that changed the meaning of the local > variable label, but left alone the call to alpha_bootblock_checksum, > thereby causing the checksum to take place over the wrong range of > bytes and be written 56 bytes past the end of the allocated memory. > The checksum call should probably just be removed as the results don't > seem to be used. > > This was discovered via a bug report against the Apline Linux package, > https://gitlab.alpinelinux.org/alpine/aports/-/issues/12161. It > appears we just got really lucky catching this, as only one value well > beyond the end of the allocation is written. It turns out that 64+512 > makes up exactly the size of musl/mallocng's next size class over 512, > 576, and writing 8 bytes before that clobbers all the consistency > check at the end of the slot and the header of the next slot. However > valgrind also seems to catch the bug when running the test cases. I had a look at this and I tried to dig up why the alpha_bootblock_checksum is called from bsd_probe() at all. I cannot see any reason why nor could git log give me any clues. I think this should be a safe fix: diff --git a/libparted/labels/bsd.c b/libparted/labels/bsd.c index 8483641..0a2b891 100644 --- a/libparted/labels/bsd.c +++ b/libparted/labels/bsd.c @@ -164,8 +164,6 @@ bsd_probe (const PedDevice *dev) label = &((BSDDiskData*) s0)->label; - alpha_bootblock_checksum(label); - /* check magic */ bool found = PED_LE32_TO_CPU (label->d_magic) == BSD_DISKMAGIC; free (s0); -nc
