[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20aa3292-091c-4d27-a77e-c0aef4bbb276@redhat.com>
Date: Thu, 24 Apr 2025 21:43:43 -0400
From: Waiman Long <llong@...hat.com>
To: Waiman Long <llong@...hat.com>, Kamaljit Singh <Kamaljit.Singh1@....com>,
"cgroups@...r.kernel.org" <cgroups@...r.kernel.org>
Cc: "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: cgroup null pointer dereference
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
Powered by blists - more mailing lists