Re: cgroup null pointer dereference

[Waiman Long]

On 4/24/25 9:33 PM, Waiman Long wrote:
> On 4/24/25 8:53 PM, Kamaljit Singh wrote:
> > Hi Waiman,
> >
> > > On 4/23/25 1:30 PM, Kamaljit Singh wrote:
> > > > Hello,
> > > >
> > > > While running IOs to an nvme fabrics target we're hitting this null 
> > > > pointer which causes
> > > > CPU hard lockups and NMI. Before the lockups, the Medusa IOs ran 
> > > > successfully for ~23 hours.
> > > >
> > > > I did not find any panics listing nvme or block driver calls.
> > > >
> > > > RIP: 0010:cgroup_rstat_flush+0x1d0/0x750
> > > > points to rstat.c, cgroup_rstat_push_children(), line 162 under 
> > > > second while() to the following code.
> > > >
> > > > 160                 /* updated_next is parent cgroup terminated */
> > > > 161                 while (child != parent) {
> > > > 162                         child->rstat_flush_next = head;
> > > > 163                         head = child;
> > > > 164                         crstatc = cgroup_rstat_cpu(child, cpu);
> > > > 165                         grandchild = crstatc->updated_children;
> > > >
> > > > In my test env I've added a null check to 'child' and re-running 
> > > > the long-term test.
> > > > I'm wondering if this patch is sufficient to address any underlying 
> > > > issue or is just a band-aid.
> > > > Please share any known patches or suggestions.
> > > >                -          while (child != parent) {
> > > >                +         while (child && child != parent) {
> > > Child can become NULL only if the updated_next list isn't parent
> > > terminated. This should not happen. A warning is needed if it really
> > > happens. I will take a further look to see if there is a bug somewhere.
> > My test re-ran for 36+ hours without any CPU lockups or NMI. This 
> > patch seems to have helped.
> I now see what is wrong. The cgroup_rstat_push_children() function is 
> supposed to be called with cgroup_rstat_lock held, but commit 
> 093c8812de2d3 ("cgroup: rstat: Cleanup flushing functions and 
> locking") changes that. Hence racing can corrupt the list. I will work 
> on a patch to fix that regression.

Oh, this problem has already been fixed in the cgroup/for-6.16 commit 
7d6c63c31914 ("cgroup: rstat: call cgroup_rstat_updated_list with 
cgroup_rstat_lock"). It should be in the linux-next tree.

Cheers,
Longman