---- 在 星期一, 2020-12-07 04:53:18 Dominique Martinet <asmadeus@xxxxxxxxxxxxx> 撰写 ---- > Dominique Martinet wrote on Sun, Dec 06, 2020: > > Chengguang Xu wrote on Sat, Dec 05, 2020: > > > If vma is shared and the file was opened for writing, > > > we should also create writeback fid because vma may be > > > mprotected writable even if now readonly. > > > > Hm, I guess it makes sense. > > I had a second look, and generic_file_readonly_mmap uses vma's > `vma->vm_flags & VM_MAYWRITE` instead (together with VM_SHARED), > while mapping_writably_mapped ultimately basically only seems to > validate that the mapping is shared from a look at mapping_map_writable > callers? It's not very clear to me. > > , VM_MAYWRITE is set anytime we have a shared map where file has > been opened read-write, which seems to be what you want with regards to > protecting from mprotect calls. > > How about simply changing check from WRITE to MAYWRITE? It would be fine and based on the code in do_mmap(), it seems we even don't need extra check here. The condition (vma->vm_flags & VM_SHARED) will be enough. Am I missing something? Thanks, Chengguang > > v9inode = V9FS_I(inode); > mutex_lock(&v9inode->v_mutex); > if (!v9inode->writeback_fid && > (vma->vm_flags & VM_SHARED) && > - (vma->vm_flags & VM_WRITE)) { > + (vma->vm_flags & VM_MAYWRITE)) { > /* > * clone a fid and add it to writeback_fid > * we do it during mmap instead of