Re: [PATCH v3] fs/proc/task_mmu: fix lost maps entries above 2^63

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Tue, Oct 13, 2026 at 05:21:06PM +0200, Stian Halseth wrote:
> When a read of /proc/pid/{maps,smaps,numa_maps} resumes, m_start()
> rewinds *ppos to priv->last_pos so that the vma which did not fit into
> the previous buffer is shown again. It only does so for last_addr > 0,
                                     ^
This could do with a newline between paragraphs.

> meaning to exclude the start of the file and SENTINEL_VMA_GATE, but
> last_addr is a signed loff_t holding a vm_end. sparc64 maps shared
                                                ^
                                          Here too.

> libraries, anonymous memory and the stack above 0x8000000000000000, so
> the rewind does not happen there and one entry is lost at every read()
> boundary.
>
> Reading the maps file of a process with read sizes from 64 to 8192
> bytes in steps of 64 loses lines at 126 of the 128 sizes, smaps at 123
> and numa_maps at 97, and the proc-maps-race selftest fails 4 of its 6
> tests. Python's sys.remote_exec() reads maps with fgets() and fails
> intermittently when it misses the libpython data mapping.

These numbers are hard to understand it's like playing numberwang a little
bit ;)

It'd be good to simplify this by saying something like 'testing revealed
that lines are not correctly read from /proc/$pid/[maps, smaps, numa_maps]
etc.' or at least something that's a little more readable.

>
> Skip the rewind only for 0 and the two sentinels instead, with an
> addr_is_sentinel() helper.
>
> Fixes: 5631da56c9a87 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
> Closes: https://github.com/sparclinux/issues/issues/111
> Cc: stable@vger.kernel.org
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Stian Halseth <stian@itx.no>

Please stop respinning so fast thanks :)

We generally want a ~week between respins for newer contributors. At least
a day.

5 minutes is a bit much...

This looks reasonable to me, but I want Suren to have a closer look.

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 5c54aebe21182..24c2a5079f080 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c

> @@ -339,7 +344,7 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
>  	 * Reset current position if last_addr was set before
>  	 * and it's not a sentinel.
>  	 */
> -	if (last_addr > 0)
> +	if (last_addr && !addr_is_sentinel(last_addr))

I think last_addr > 0 was fine, but it's not really a big deal.

--
Cheers, Lorenzo