On Mon, Dec 7, 2015 at 2:42 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > On Thu, Dec 3, 2015 at 5:45 PM, yalin wang <yalin.wang2010@xxxxxxxxx> wrote: >> >>> On Dec 2, 2015, at 16:03, Kees Cook <keescook@xxxxxxxxxxxx> wrote: >>> >>> Normally, when a user can modify a file that has setuid or setgid bits, >>> those bits are cleared when they are not the file owner or a member >>> of the group. This is enforced when using write and truncate but not >>> when writing to a shared mmap on the file. This could allow the file >>> writer to gain privileges by changing a binary without losing the >>> setuid/setgid/caps bits. >>> >>> Changing the bits requires holding inode->i_mutex, so it cannot be done >>> during the page fault (due to mmap_sem being held during the fault). >>> Instead, clear the bits if PROT_WRITE is being used at mmap time. >>> >>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> >>> Cc: stable@xxxxxxxxxxxxxxx >>> — >> >> is this means mprotect() sys call also need add this check? >> mprotect() can change to PROT_WRITE, then it can write to a >> read only map again , also a secure hole here . > > Yes, good point. This needs to be added. I will send a new patch. Thanks! This continues to look worse and worse. So... to check this at mprotect time, I have to know it's MAP_SHARED, but that's in the vma_flags, which I can only see after holding mmap_sem. The best I can think of now is to strip the bits at munmap time, since you can't execute an mmapped file until it closes. Jan, thoughts on this? -Kees -- Kees Cook Chrome OS & Brillo Security -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href