Messages in this thread | | | Date | Fri, 15 Sep 2023 11:44:40 -0400 | From | Ben Wolsieffer <> | Subject | Re: [PATCH] proc: nommu: /proc/<pid>/maps: release mmap read lock |
| |
On Fri, Sep 15, 2023 at 02:15:15PM +0200, Oleg Nesterov wrote: > Sorry for the offtopic question. I know NOTHING about nommu and when I tried to > review this patch I was puzzled by > > /* See m_next(). Zero at the start or after lseek. */ > if (addr == -1UL) > return NULL; > > at the start of m_start(). OK, lets look at > > static void *m_next(struct seq_file *m, void *_p, loff_t *pos) > { > struct vm_area_struct *vma = _p; > > *pos = vma->vm_end; > return find_vma(vma->vm_mm, vma->vm_end); > } > > where does this -1UL come from? Does this mean that on nommu > > last_vma->vm_end == -1UL > > or what? > > fs/proc/task_mmu.c has the same check at the start, but in this case > the "See m_next()" comment actually helps.
Yes, this is another copying mistake from the MMU implementation. In fact, it turns out that no-MMU /proc/<pid>/maps is completely broken after 0c563f148043 ("proc: remove VMA rbtree use from nommu"). It just returns an empty file.
This happens because find_vma() doesn't do what we want here. It "look[s] up the first VMA in which addr resides, NULL if none", and the address will be zero in in m_start(), which makes find_vma() return NULL (unless presumably the zero address is actually part of the process's address space).
I didn't run into this because I developed my patch against an older kernel, and didn't test the latest version until today.
I'm preparing a second patch to fix this bug.
> > Just curious, thanks. > > Oleg. >
Thanks, Ben
| |