Re: [RFC PATCH v25 07/14] mm: keep the RSS counters of single-threaded processes exact

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

> [Severity: Medium]
> Can a failed fork leave mm->rss_owner pointing to a freed task_struct?

[...]

> If an external reference holds mm->mm_users > 1 across that mmput(), mm
> outlives tsk.  A new task allocated at the same slab address as tsk can
> then match current == READ_ONCE(mm->rss_owner) in __add_mm_counter() and
> perform unsynchronized non-atomic updates via
> percpu_counter_tree_add_owner(), losing counter updates or triggering a
> Bad rss-counter state report in check_mm().
>
> Clearing mm->rss_owner alongside mm->owner on both error paths before
> mmput() would avoid leaving the dangling pointer.

Yes, both error paths leave rss_owner pointing to the child. This was
found by reading the code, I did not reproduce it.

No update would be lost, though. rss_owner is only compared with
current, never dereferenced. At most one live task has that address at a
time, so the plain counter still has a single writer. The other tasks
update the atomic counter. The counters would still work.

What breaks is the identity of the owner: a task which does not run on
the mm would update the plain counter as its owner. The owner should
stay the task running on the mm.

Planned fix for the next round: clear mm->rss_owner next to mm->owner on
both error paths. In copy_process() this is done only when rss_owner is
the child, because with CLONE_VM the mm is the parent's.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com