On Thu, Jul 13, 2023 at 03:57:00PM +0800, linke li wrote: > > However, if this is a real issue it would make more > > sense to look for and change all such checks rather than one single occurrence. > > Hi, Mike. I have checked the example code you provided, and the > difference between > those codes and the patched code is that those checks are checks for > unsigned integer > overflow, which is well-defined. Only undefined behavior poses a > security risk. So they > don't need any modifications. I have only found one occurrence of > signed number > overflow so far. I used to have a similar check to that but I eventually deleted it because I decided that the -fno-strict-overflow option works. It didn't produce a lot of warnings. Historically we have done a bad job at open coding integer overflow checks. Some that I wrote turned out to be incorrect. And even when I write them correctly a couple times people have "fixed" them even harder without CCing me or asking me why I wrote them the way I did. What about using the check_add_overflow() macro? diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index 7b17ccfa039d..c512165736e0 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -155,9 +155,8 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma) return -EINVAL; vma_len = (loff_t)(vma->vm_end - vma->vm_start); - len = vma_len + ((loff_t)vma->vm_pgoff << PAGE_SHIFT); - /* check for overflow */ - if (len < vma_len) + if (check_add_overflow(vma_len, (loff_t)vma->vm_pgoff << PAGE_SHIFT, + &len)) return -EINVAL; inode_lock(inode);
