Re: [PATCH] psi: fix monitor for root cgroup

2020-12-08 Thread Suren Baghdasaryan
On Tue, Dec 8, 2020 at 12:35 AM 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 1 100"
>
>
> Signed-off-by: Odin Ugedal 
> ---
>  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 e41c21819ba0..5d1fdf7c3ec6 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -3567,6 +3567,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)
> @@ -3575,7 +3576,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.29.2
>

Reviewed-by: Suren Baghdasaryan 


[PATCH] psi: fix monitor for root cgroup

2020-12-08 Thread 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 1 100"


Signed-off-by: Odin Ugedal 
---
 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 e41c21819ba0..5d1fdf7c3ec6 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3567,6 +3567,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)
@@ -3575,7 +3576,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.29.2