On 3/7/20 8:52 PM, Changming Liu wrote: > Hi Jens, > Greetings, I'm a first-year PhD student who is interested in the usage > of UBSan in linux kernel. With some experiments, I found that in > /block/ioctl.c function blk_ioctl_discard. > > Two uint64 integers, namely, start and len, are directly from user > space, so the sum of these two can overflow and wrap around. As a > consequence, the check of the sum against function i_size_read at if > (start + len > i_size_read(bdev->bd_inode)) can be skipped due to the > unsigned wrap around, the overflown sum is passed to the 3rd parameter > of function truncate_inode_pages_range, which might cause undesired > issue. This still exists in the latest version, i.e. linux-5.5.8. > > It's well worth noting that, a very similar pattern can be witnessed > in function blk_ioctl_zeroout where there are also two uint64 > variables with the same name from user space, and the sum of the two > variables are passed to function truncate_inode_pages_range too. > However in this case, the wrap around is check at line 262, thus the > value passed to truncate_inode_pages_range cannot overflow. > > So it looks like the issue in blk_ioctl_zeroout was discussed and > fixed in http://lkml.iu.edu/hypermail/linux/kernel/1511.1/04403.html > But since in blk_ioctl_discard has the same issue, I wonder if it's > worth fixing the issue in blk_ioctl_discard as well. If not, I would > appreciate it if I can know the reason, this can help me understand > the system a lot. > > I cc my colleague on the experiment here to keep him updated. > > It's a great honor to reach out to you hardcore linux kernel > developer, you guys have been the hero ever since I started learning > CS. Looking forward to your valuable response! Looks like your analysis is correct, care to send a patch to fix it up? -- Jens Axboe
