On Aug 8, 2019, at 1:12 AM, Carlos Maiolino <cmaiolino@xxxxxxxxxx> wrote: > >>> >>>> Maybe I am not seeing something or having a different thinking you have, but >>>> this is the behavior we have now, without my patches. And we can't really change >>>> it; the user view of this implementation. >>>> That's why I didn't try to change the result, so the truncation still happens. >>> >>> I understand that we're not generally supposed to change existing >>> userspace interfaces, but the fact remains that allowing truncated >>> responses causes *filesystem corruption*. >>> >>> We know that the most well known FIBMAP callers are bootloaders, and we >>> know what they do with the information they get -- they use it to record >>> the block map of boot files. So if the IPL/grub/whatever installer >>> queries the boot file and the boot file is at block 12345678901 (a >>> 34-bit number), this interface truncates that to 3755744309 (a 32-bit >>> number) and that's where the bootloader will think its boot files are. >>> The installation succeeds, the user reboots and *kaboom* the system no >>> longer boots because the contents of block 3755744309 is not a bootloader. >>> >>> Worse yet, grub1 used FIBMAP data to record the location of the grub >>> environment file and installed itself between the MBR and the start of >>> partition 1. If the environment file is at offset 1234578901, grub will >>> write status data to its environment file (which it thinks is at >>> 3755744309) and *KABOOM* we've just destroyed whatever was in that >>> block. >>> >>> Far better for the bootloader installation script to hit an error and >>> force the admin to deal with the situation than for the system to become >>> unbootable. That's *why* the (newer) iomap bmap implementation does not >>> return truncated mappings, even though the classic implementation does. >>> >>> The classic code returning truncated results is a broken behavior. >> >> How long as it been broken for? And if we do fix it, I'd just like for >> a nice commit lot describing potential risks of not applying it. *If* >> the issue exists as-is today, the above contains a lot of information >> for addressing potential issues, even if theoretical. >> > > It's broken since forever. This has always been the FIBMAP behavior. It's been broken since forever, but only for filesystems larger than 4TB or 16TB (2^32 blocks), which are only becoming commonplace for root disks recently. Also, doesn't LILO have a limit on the location of the kernel image, in the first 1GB or similar? So maybe this is not an issue that FIBMAP users ever hit in practise anyway, but I agree that it doesn't make sense to return bad data (32-bit wrapped block numbers) and 0 should be returned in such cases. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
