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

[David Hildenbrand (Arm)]

On 10/11/26 18:20, 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,
> 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
> 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.
> 
> Test for the two values that are not a vma end instead.
> 
> Fixes: 5631da56c9a87 ("fs/proc/task_mmu: read proc/pid/maps under per-vma lock")
> Closes: https://github.com/sparclinux/issues/issues/111
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Stian Halseth <stian@xxxxxx>
> ---
> 
> Notes:
>     Tested on a SPARC T7-1 (sun4v, 7.3-rc5), a T4-1 (7.2.9) and a Sun Fire
>     V240 (sun4u, 7.3-rc5): no lost lines at any read size afterwards, and
>     Python's sys.remote_exec() test passes 20/20. On an amd64 KVM guest maps,
>     smaps, numa_maps and the proc selftests are unchanged.
> 
>  fs/proc/task_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 5c54aebe21182..cdce513edcb1d 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -339,7 +339,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 && last_addr != SENTINEL_VMA_GATE)
>  		*ppos = last_addr = priv->last_pos;
>  	vma_iter_init(&priv->iter, mm, (unsigned long)last_addr);
>  	hold_task_mempolicy(priv);

I hope Suren can review as well.

Would it be cleaner to reject any special values, that is, SENTINEL_VMA_END and
SENTINEL_VMA_GATE?

That would match the comment and the previous code idea: exclude only 0 and
sentinels.

Could consider a simple addr_is_sentinel() helper.

-- 
Cheers,

David