On 07/11/23 13:49, David Hildenbrand wrote: > On 10.07.23 10:32, Linke Li wrote: > > From: Linke Li <lilinke99@xxxxxxxxx> > > > > 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) > > return -EINVAL; > > > > The existing code includes an integer overflow check, which indicates > > that the variable len has the potential to overflow, leading to undefined > > behavior according to the C standard. However, both GCC and Clang > > compilers may eliminate this overflow check based on the assumption > > that there will be no undefined behavior. Although the Linux kernel > > disables these optimizations by using the -fno-strict-overflow option, > > there is still a risk if the compilers make mistakes in the future. > > So we're adding code to handle eventual future compiler bugs? That sounds > wrong, but maybe I misunderstood the problem you are trying to solve? Like David, adding a fix for a potential future compiler bug sounds wrong. I have no problem with restructuring code to make it more immune to potential issues. However, it appears there are several places throughout the kernel that perform similar checks. For example: do_mmap() /* offset overflow? */ if ((pgoff + (len >> PAGE_SHIFT)) < pgoff) return -EOVERFLOW; expand_upwards() /* Enforce stack_guard_gap */ gap_addr = address + stack_guard_gap; /* Guard against overflow */ if (gap_addr < address || gap_addr > TASK_SIZE) gap_addr = TASK_SIZE; do_madvise() end = start + len; if (end < start) return -EINVAL; I am not suggesting that these all be changed. The question of a real issue still remains. 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. -- Mike Kravetz
