* [PATCH v2 0/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-16 17:36 ` Odin Ugedal
0 siblings, 0 replies; 14+ messages in thread
From: Odin Ugedal @ 2021-01-16 17:36 UTC (permalink / raw)
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan-hv44wF8Li93QT0dZR+AlfA,
hannes-druUgvl0LCNAfugRpC6u6w, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dschatzberg-b10kYP2dOMg,
surenb-hpIqsD4AKlfQT0dZR+AlfA, Odin Ugedal
This patchset fixes PSI monitors on the root cgroup, as they currently
only works on the non-root cgroups. Reading works for all, since that
was fixed when adding support for the root PSI files. It also contains
a doc update to reflect the current implementation.
Changes since v1:
- Added Reviewed-by tag on the original patch (Suren)
- Updated patch title
- Added a separate patch to update doc
Odin Ugedal (2):
cgroup: fix psi monitor for root cgroup
cgroup: update PSI file description in docs
Documentation/admin-guide/cgroup-v2.rst | 6 +++---
kernel/cgroup/cgroup.c | 4 +++-
2 files changed, 6 insertions(+), 4 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-16 17:36 ` Odin Ugedal
0 siblings, 0 replies; 14+ messages in thread
From: Odin Ugedal @ 2021-01-16 17:36 UTC (permalink / raw)
Cc: tj, lizefan, hannes, cgroups, linux-kernel, dschatzberg, surenb,
Odin Ugedal
Fix NULL pointer dereference when adding new psi monitor to the root
cgroup. PSI files for root cgroup was introduced in df5ba5be742 by using
system wide psi struct when reading, but file write/monitor was not
properly fixed. Since the PSI config for the root cgroup isn't
initialized, the current implementation tries to lock a NULL ptr,
resulting in a crash.
Can be triggered by running this as root:
$ tee /sys/fs/cgroup/cpu.pressure <<< "some 10000 1000000"
Signed-off-by: Odin Ugedal <odin@uged.al>
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
---
kernel/cgroup/cgroup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 613845769103..1ea995f801ec 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3564,6 +3564,7 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
{
struct psi_trigger *new;
struct cgroup *cgrp;
+ struct psi_group *psi;
cgrp = cgroup_kn_lock_live(of->kn, false);
if (!cgrp)
@@ -3572,7 +3573,8 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
cgroup_get(cgrp);
cgroup_kn_unlock(of->kn);
- new = psi_trigger_create(&cgrp->psi, buf, nbytes, res);
+ psi = cgroup_ino(cgrp) == 1 ? &psi_system : &cgrp->psi;
+ new = psi_trigger_create(psi, buf, nbytes, res);
if (IS_ERR(new)) {
cgroup_put(cgrp);
return PTR_ERR(new);
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 1/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-16 17:36 ` Odin Ugedal
0 siblings, 0 replies; 14+ messages in thread
From: Odin Ugedal @ 2021-01-16 17:36 UTC (permalink / raw)
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan-hv44wF8Li93QT0dZR+AlfA,
hannes-druUgvl0LCNAfugRpC6u6w, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dschatzberg-b10kYP2dOMg,
surenb-hpIqsD4AKlfQT0dZR+AlfA, Odin Ugedal
Fix NULL pointer dereference when adding new psi monitor to the root
cgroup. PSI files for root cgroup was introduced in df5ba5be742 by using
system wide psi struct when reading, but file write/monitor was not
properly fixed. Since the PSI config for the root cgroup isn't
initialized, the current implementation tries to lock a NULL ptr,
resulting in a crash.
Can be triggered by running this as root:
$ tee /sys/fs/cgroup/cpu.pressure <<< "some 10000 1000000"
Signed-off-by: Odin Ugedal <odin-RObV4cXtwVA@public.gmane.org>
Reviewed-by: Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
kernel/cgroup/cgroup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 613845769103..1ea995f801ec 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3564,6 +3564,7 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
{
struct psi_trigger *new;
struct cgroup *cgrp;
+ struct psi_group *psi;
cgrp = cgroup_kn_lock_live(of->kn, false);
if (!cgrp)
@@ -3572,7 +3573,8 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
cgroup_get(cgrp);
cgroup_kn_unlock(of->kn);
- new = psi_trigger_create(&cgrp->psi, buf, nbytes, res);
+ psi = cgroup_ino(cgrp) == 1 ? &psi_system : &cgrp->psi;
+ new = psi_trigger_create(psi, buf, nbytes, res);
if (IS_ERR(new)) {
cgroup_put(cgrp);
return PTR_ERR(new);
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 16:52 ` Johannes Weiner
0 siblings, 0 replies; 14+ messages in thread
From: Johannes Weiner @ 2021-01-19 16:52 UTC (permalink / raw)
To: Odin Ugedal; +Cc: tj, lizefan, cgroups, linux-kernel, dschatzberg, surenb
On Sat, Jan 16, 2021 at 06:36:33PM +0100, Odin Ugedal wrote:
> Fix NULL pointer dereference when adding new psi monitor to the root
> cgroup. PSI files for root cgroup was introduced in df5ba5be742 by using
> system wide psi struct when reading, but file write/monitor was not
> properly fixed. Since the PSI config for the root cgroup isn't
> initialized, the current implementation tries to lock a NULL ptr,
> resulting in a crash.
>
> Can be triggered by running this as root:
> $ tee /sys/fs/cgroup/cpu.pressure <<< "some 10000 1000000"
>
> Signed-off-by: Odin Ugedal <odin@uged.al>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Fixes: df5ba5be7425 ("kernel/sched/psi.c: expose pressure metrics on root cgroup")
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Since this is a userspace-triggerable NULL ptr crash, we should
probably also
Cc: stable@vger.kernel.org # 5.2+
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 16:52 ` Johannes Weiner
0 siblings, 0 replies; 14+ messages in thread
From: Johannes Weiner @ 2021-01-19 16:52 UTC (permalink / raw)
To: Odin Ugedal
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan-hv44wF8Li93QT0dZR+AlfA,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dschatzberg-b10kYP2dOMg,
surenb-hpIqsD4AKlfQT0dZR+AlfA
On Sat, Jan 16, 2021 at 06:36:33PM +0100, Odin Ugedal wrote:
> Fix NULL pointer dereference when adding new psi monitor to the root
> cgroup. PSI files for root cgroup was introduced in df5ba5be742 by using
> system wide psi struct when reading, but file write/monitor was not
> properly fixed. Since the PSI config for the root cgroup isn't
> initialized, the current implementation tries to lock a NULL ptr,
> resulting in a crash.
>
> Can be triggered by running this as root:
> $ tee /sys/fs/cgroup/cpu.pressure <<< "some 10000 1000000"
>
> Signed-off-by: Odin Ugedal <odin-RObV4cXtwVA@public.gmane.org>
> Reviewed-by: Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Fixes: df5ba5be7425 ("kernel/sched/psi.c: expose pressure metrics on root cgroup")
Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
Since this is a userspace-triggerable NULL ptr crash, we should
probably also
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 5.2+
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] cgroup: update PSI file description in docs
@ 2021-01-16 17:36 ` Odin Ugedal
0 siblings, 0 replies; 14+ messages in thread
From: Odin Ugedal @ 2021-01-16 17:36 UTC (permalink / raw)
Cc: tj, lizefan, hannes, cgroups, linux-kernel, dschatzberg, surenb,
Odin Ugedal
Update PSI file description in cgroup-v2 docs to reflect the current
implementation.
Signed-off-by: Odin Ugedal <odin@uged.al>
---
Documentation/admin-guide/cgroup-v2.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 63521cd36ce5..f638c9d3d9f2 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1029,7 +1029,7 @@ All time durations are in microseconds.
one number is written, $MAX is updated.
cpu.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for CPU. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
@@ -1475,7 +1475,7 @@ PAGE_SIZE multiple when read back.
reduces the impact on the workload and memory management.
memory.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for memory. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
@@ -1714,7 +1714,7 @@ IO Interface Files
8:16 rbps=2097152 wbps=max riops=max wiops=max
io.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for IO. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] cgroup: update PSI file description in docs
@ 2021-01-16 17:36 ` Odin Ugedal
0 siblings, 0 replies; 14+ messages in thread
From: Odin Ugedal @ 2021-01-16 17:36 UTC (permalink / raw)
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan-hv44wF8Li93QT0dZR+AlfA,
hannes-druUgvl0LCNAfugRpC6u6w, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dschatzberg-b10kYP2dOMg,
surenb-hpIqsD4AKlfQT0dZR+AlfA, Odin Ugedal
Update PSI file description in cgroup-v2 docs to reflect the current
implementation.
Signed-off-by: Odin Ugedal <odin-RObV4cXtwVA@public.gmane.org>
---
Documentation/admin-guide/cgroup-v2.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 63521cd36ce5..f638c9d3d9f2 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1029,7 +1029,7 @@ All time durations are in microseconds.
one number is written, $MAX is updated.
cpu.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for CPU. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
@@ -1475,7 +1475,7 @@ PAGE_SIZE multiple when read back.
reduces the impact on the workload and memory management.
memory.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for memory. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
@@ -1714,7 +1714,7 @@ IO Interface Files
8:16 rbps=2097152 wbps=max riops=max wiops=max
io.pressure
- A read-only nested-key file which exists on non-root cgroups.
+ A read-only nested-keyed file.
Shows pressure stall information for IO. See
:ref:`Documentation/accounting/psi.rst <psi>` for details.
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] cgroup: update PSI file description in docs
2021-01-16 17:36 ` Odin Ugedal
(?)
@ 2021-01-19 16:56 ` Johannes Weiner
2021-01-19 17:03 ` Tejun Heo
-1 siblings, 1 reply; 14+ messages in thread
From: Johannes Weiner @ 2021-01-19 16:56 UTC (permalink / raw)
To: Odin Ugedal; +Cc: tj, lizefan, cgroups, linux-kernel, dschatzberg, surenb
On Sat, Jan 16, 2021 at 06:36:34PM +0100, Odin Ugedal wrote:
> Update PSI file description in cgroup-v2 docs to reflect the current
> implementation.
>
> Signed-off-by: Odin Ugedal <odin@uged.al>
> ---
> Documentation/admin-guide/cgroup-v2.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index 63521cd36ce5..f638c9d3d9f2 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -1029,7 +1029,7 @@ All time durations are in microseconds.
> one number is written, $MAX is updated.
>
> cpu.pressure
> - A read-only nested-key file which exists on non-root cgroups.
> + A read-only nested-keyed file.
Could you please also change the 'read-only' to 'read-write'?
With that, please feel free to add:
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] cgroup: update PSI file description in docs
2021-01-19 16:56 ` Johannes Weiner
@ 2021-01-19 17:03 ` Tejun Heo
0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2021-01-19 17:03 UTC (permalink / raw)
To: Johannes Weiner
Cc: Odin Ugedal, lizefan, cgroups, linux-kernel, dschatzberg, surenb
On Tue, Jan 19, 2021 at 11:56:18AM -0500, Johannes Weiner wrote:
> On Sat, Jan 16, 2021 at 06:36:34PM +0100, Odin Ugedal wrote:
> > Update PSI file description in cgroup-v2 docs to reflect the current
> > implementation.
> >
> > Signed-off-by: Odin Ugedal <odin@uged.al>
> > ---
> > Documentation/admin-guide/cgroup-v2.rst | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > index 63521cd36ce5..f638c9d3d9f2 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > @@ -1029,7 +1029,7 @@ All time durations are in microseconds.
> > one number is written, $MAX is updated.
> >
> > cpu.pressure
> > - A read-only nested-key file which exists on non-root cgroups.
> > + A read-only nested-keyed file.
>
> Could you please also change the 'read-only' to 'read-write'?
>
> With that, please feel free to add:
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Applied w/ the suggested change.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 16:46 ` Dan Schatzberg
0 siblings, 0 replies; 14+ messages in thread
From: Dan Schatzberg @ 2021-01-19 16:46 UTC (permalink / raw)
To: Odin Ugedal; +Cc: tj, lizefan, hannes, cgroups, linux-kernel, surenb
> This patchset fixes PSI monitors on the root cgroup, as they currently
> only works on the non-root cgroups. Reading works for all, since that
> was fixed when adding support for the root PSI files. It also contains
> a doc update to reflect the current implementation.
>
> Changes since v1:
> - Added Reviewed-by tag on the original patch (Suren)
> - Updated patch title
> - Added a separate patch to update doc
>
>
> Odin Ugedal (2):
> cgroup: fix psi monitor for root cgroup
> cgroup: update PSI file description in docs
>
> Documentation/admin-guide/cgroup-v2.rst | 6 +++---
> kernel/cgroup/cgroup.c | 4 +++-
> 2 files changed, 6 insertions(+), 4 deletions(-)
Both patches look good.
Acked-by: Dan Schatzberg <dschatzberg@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 16:46 ` Dan Schatzberg
0 siblings, 0 replies; 14+ messages in thread
From: Dan Schatzberg @ 2021-01-19 16:46 UTC (permalink / raw)
To: Odin Ugedal
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, lizefan-hv44wF8Li93QT0dZR+AlfA,
hannes-druUgvl0LCNAfugRpC6u6w, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
surenb-hpIqsD4AKlfQT0dZR+AlfA
> This patchset fixes PSI monitors on the root cgroup, as they currently
> only works on the non-root cgroups. Reading works for all, since that
> was fixed when adding support for the root PSI files. It also contains
> a doc update to reflect the current implementation.
>
> Changes since v1:
> - Added Reviewed-by tag on the original patch (Suren)
> - Updated patch title
> - Added a separate patch to update doc
>
>
> Odin Ugedal (2):
> cgroup: fix psi monitor for root cgroup
> cgroup: update PSI file description in docs
>
> Documentation/admin-guide/cgroup-v2.rst | 6 +++---
> kernel/cgroup/cgroup.c | 4 +++-
> 2 files changed, 6 insertions(+), 4 deletions(-)
Both patches look good.
Acked-by: Dan Schatzberg <dschatzberg-b10kYP2dOMg@public.gmane.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 17:00 ` Tejun Heo
0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2021-01-19 17:00 UTC (permalink / raw)
To: Odin Ugedal; +Cc: lizefan, hannes, cgroups, linux-kernel, dschatzberg, surenb
On Sat, Jan 16, 2021 at 06:36:32PM +0100, Odin Ugedal wrote:
> This patchset fixes PSI monitors on the root cgroup, as they currently
> only works on the non-root cgroups. Reading works for all, since that
> was fixed when adding support for the root PSI files. It also contains
> a doc update to reflect the current implementation.
Applied to cgroup/for-5.11-fixes with acks and stable tag added.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] cgroup: fix psi monitor for root cgroup
@ 2021-01-19 17:00 ` Tejun Heo
0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2021-01-19 17:00 UTC (permalink / raw)
To: Odin Ugedal
Cc: lizefan-hv44wF8Li93QT0dZR+AlfA, hannes-druUgvl0LCNAfugRpC6u6w,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dschatzberg-b10kYP2dOMg,
surenb-hpIqsD4AKlfQT0dZR+AlfA
On Sat, Jan 16, 2021 at 06:36:32PM +0100, Odin Ugedal wrote:
> This patchset fixes PSI monitors on the root cgroup, as they currently
> only works on the non-root cgroups. Reading works for all, since that
> was fixed when adding support for the root PSI files. It also contains
> a doc update to reflect the current implementation.
Applied to cgroup/for-5.11-fixes with acks and stable tag added.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 14+ messages in thread