[Devel] [PATCH RH7] proc/mounts: fix skipping mount after cursor

[Alexander Mikhalitsyn]

On Fri, 26 Mar 2021 15:15:47 +0300
Pavel Tikhomirov <ptikhomirov at virtuozzo.com> wrote:

> When reading mounts with:
> 
>   mount -t tmpfs $(printf 'T%.0s' {1..76}) /tmp/testmount
>   tail -n1 /proc/self/mountinfo | wc
>   #      1      10     128
>   # len should be 128
>   while read -r line; do echo $line; done < /proc/<pid>/mountinfo
> 
> We can see that after TTT mount we stuck on it and show it infinitely.
> 
> - Let's fix the case where we do m_start() and *pos == 0, in this case
> we need to reset cursor caches (last_pos and last_mntpos) as user asks
> us to start iterating from the beginning and we don't need to skip
> any mount based on these caches.
> 
> - Let's set last_mntpos in m_stop and only in m_stop to actually make it
> cache the mount after cursor. There is no point to set it in m_next as
> cursor is inserted only in m_stop. Also let's reset last_mntpos to zero
> if cursor is removed for consistency.
> 
> - Let's fix last_mntpos != mnt check in m_start, in case last_mntpos is
> unset we does not want to skip any mount.
> 
> - Let's update last_pos in the end of m_start like in m_next. Where is a
> posibility that in seq_read we print one entry without calling m_next at
> all and go directly to m_stop and increment pos. If previously we had
> pos == last_poss, after first m_start pos would be == last_pos + 1, and
> after second m_start pos would be == last_pos + 2 and we would forget to
> skip mount and will be printing it forever infinitely.
> 
> https://jira.sw.ru/browse/PSBM-127476
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

Reviewed-by: Alexander Mikhalitsyn <alexander.mikhalitsyn at virtuozzo.com>

> ---
>  fs/namespace.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 042ede7abc3b6..a1c5afb446f05 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1457,6 +1457,8 @@ static void *m_start(struct seq_file *m, loff_t *pos)
>  	down_read(&namespace_sem);
>  	if (!*pos) {
>  		prev = &p->ns->list;
> +		p->last_pos = 0;
> +		p->last_mntpos = NULL;
>  	} else {
>  		prev = &p->cursor.mnt_list;
>  
> @@ -1509,10 +1511,10 @@ static void *m_start(struct seq_file *m, loff_t *pos)
>  	 * here and just remove following lines and p->last_pos field.
>  	 */
>  	if (mnt && (*pos == p->last_pos + 1) &&
> -	    !(p->last_mntpos && (p->last_mntpos != mnt)))
> +	    (p->last_mntpos == mnt))
>  		mnt = mnt_list_next(p->ns, &mnt->mnt_list);
>  
> -	p->last_mntpos = mnt;
> +	p->last_pos = *pos;
>  
>  	return mnt;
>  }
> @@ -1524,8 +1526,7 @@ static void *m_next(struct seq_file *m, void *v, loff_t *pos)
>  
>  	++*pos;
>  	p->last_pos = *pos;
> -	p->last_mntpos = mnt_list_next(p->ns, &mnt->mnt_list);
> -	return p->last_mntpos;
> +	return mnt_list_next(p->ns, &mnt->mnt_list);
>  }
>  
>  static void m_stop(struct seq_file *m, void *v)
> @@ -1534,10 +1535,13 @@ static void m_stop(struct seq_file *m, void *v)
>  	struct mount *mnt = v;
>  
>  	lock_ns_list(p->ns);
> -	if (mnt)
> +	if (mnt) {
>  		list_move_tail(&p->cursor.mnt_list, &mnt->mnt_list);
> -	else
> +		p->last_mntpos = mnt;
> +	} else {
>  		list_del_init(&p->cursor.mnt_list);
> +		p->last_mntpos = NULL;
> +	}
>  	unlock_ns_list(p->ns);
>  	up_read(&namespace_sem);
>  }
> -- 
> 2.30.2
>