On Sat, Jan 29, 2022 at 4:49 AM <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
>
> The patch below does not apply to the 5.10-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@xxxxxxxxxxxxxxx>.
Thanks for letting me know! I'll try backporting to 5.4 and 5.10. Will
post patches later today.
Thanks,
Suren.
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From a06247c6804f1a7c86a2e5398a4c1f1db1471848 Mon Sep 17 00:00:00 2001
> From: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Date: Tue, 11 Jan 2022 15:23:09 -0800
> Subject: [PATCH] psi: Fix uaf issue when psi trigger is destroyed while being
> polled
>
> With write operation on psi files replacing old trigger with a new one,
> the lifetime of its waitqueue is totally arbitrary. Overwriting an
> existing trigger causes its waitqueue to be freed and pending poll()
> will stumble on trigger->event_wait which was destroyed.
> Fix this by disallowing to redefine an existing psi trigger. If a write
> operation is used on a file descriptor with an already existing psi
> trigger, the operation will fail with EBUSY error.
> Also bypass a check for psi_disabled in the psi_trigger_destroy as the
> flag can be flipped after the trigger is created, leading to a memory
> leak.
>
> Fixes: 0e94682b73bf ("psi: introduce psi monitor")
> Reported-by: syzbot+cdb5dd11c97cc532efad@xxxxxxxxxxxxxxxxxxxxxxxxx
> Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Analyzed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Reviewed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/r/20220111232309.1786347-1-surenb@xxxxxxxxxx
>
> diff --git a/Documentation/accounting/psi.rst b/Documentation/accounting/psi.rst
> index f2b3439edcc2..860fe651d645 100644
> --- a/Documentation/accounting/psi.rst
> +++ b/Documentation/accounting/psi.rst
> @@ -92,7 +92,8 @@ Triggers can be set on more than one psi metric and more than one trigger
> for the same psi metric can be specified. However for each trigger a separate
> file descriptor is required to be able to poll it separately from others,
> therefore for each trigger a separate open() syscall should be made even
> -when opening the same psi interface file.
> +when opening the same psi interface file. Write operations to a file descriptor
> +with an already existing psi trigger will fail with EBUSY.
>
> Monitors activate only when system enters stall state for the monitored
> psi metric and deactivates upon exit from the stall state. While system is
> diff --git a/include/linux/psi.h b/include/linux/psi.h
> index a70ca833c6d7..f8ce53bfdb2a 100644
> --- a/include/linux/psi.h
> +++ b/include/linux/psi.h
> @@ -33,7 +33,7 @@ void cgroup_move_task(struct task_struct *p, struct css_set *to);
>
> struct psi_trigger *psi_trigger_create(struct psi_group *group,
> char *buf, size_t nbytes, enum psi_res res);
> -void psi_trigger_replace(void **trigger_ptr, struct psi_trigger *t);
> +void psi_trigger_destroy(struct psi_trigger *t);
>
> __poll_t psi_trigger_poll(void **trigger_ptr, struct file *file,
> poll_table *wait);
> diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h
> index 516c0fe836fd..1a3cef26d129 100644
> --- a/include/linux/psi_types.h
> +++ b/include/linux/psi_types.h
> @@ -141,9 +141,6 @@ struct psi_trigger {
> * events to one per window
> */
> u64 last_event_time;
> -
> - /* Refcounting to prevent premature destruction */
> - struct kref refcount;
> };
>
> struct psi_group {
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index b31e1465868a..9d05c3ca2d5e 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -3643,6 +3643,12 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
> cgroup_get(cgrp);
> cgroup_kn_unlock(of->kn);
>
> + /* Allow only one trigger per file descriptor */
> + if (ctx->psi.trigger) {
> + cgroup_put(cgrp);
> + return -EBUSY;
> + }
> +
> psi = cgroup_ino(cgrp) == 1 ? &psi_system : &cgrp->psi;
> new = psi_trigger_create(psi, buf, nbytes, res);
> if (IS_ERR(new)) {
> @@ -3650,8 +3656,7 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
> return PTR_ERR(new);
> }
>
> - psi_trigger_replace(&ctx->psi.trigger, new);
> -
> + smp_store_release(&ctx->psi.trigger, new);
> cgroup_put(cgrp);
>
> return nbytes;
> @@ -3690,7 +3695,7 @@ static void cgroup_pressure_release(struct kernfs_open_file *of)
> {
> struct cgroup_file_ctx *ctx = of->priv;
>
> - psi_trigger_replace(&ctx->psi.trigger, NULL);
> + psi_trigger_destroy(ctx->psi.trigger);
> }
>
> bool cgroup_psi_enabled(void)
> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
> index a679613a7cb7..c137c4d6983e 100644
> --- a/kernel/sched/psi.c
> +++ b/kernel/sched/psi.c
> @@ -1162,7 +1162,6 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
> t->event = 0;
> t->last_event_time = 0;
> init_waitqueue_head(&t->event_wait);
> - kref_init(&t->refcount);
>
> mutex_lock(&group->trigger_lock);
>
> @@ -1191,15 +1190,19 @@ struct psi_trigger *psi_trigger_create(struct psi_group *group,
> return t;
> }
>
> -static void psi_trigger_destroy(struct kref *ref)
> +void psi_trigger_destroy(struct psi_trigger *t)
> {
> - struct psi_trigger *t = container_of(ref, struct psi_trigger, refcount);
> - struct psi_group *group = t->group;
> + struct psi_group *group;
> struct task_struct *task_to_destroy = NULL;
>
> - if (static_branch_likely(&psi_disabled))
> + /*
> + * We do not check psi_disabled since it might have been disabled after
> + * the trigger got created.
> + */
> + if (!t)
> return;
>
> + group = t->group;
> /*
> * Wakeup waiters to stop polling. Can happen if cgroup is deleted
> * from under a polling process.
> @@ -1235,9 +1238,9 @@ static void psi_trigger_destroy(struct kref *ref)
> mutex_unlock(&group->trigger_lock);
>
> /*
> - * Wait for both *trigger_ptr from psi_trigger_replace and
> - * poll_task RCUs to complete their read-side critical sections
> - * before destroying the trigger and optionally the poll_task
> + * Wait for psi_schedule_poll_work RCU to complete its read-side
> + * critical section before destroying the trigger and optionally the
> + * poll_task.
> */
> synchronize_rcu();
> /*
> @@ -1254,18 +1257,6 @@ static void psi_trigger_destroy(struct kref *ref)
> kfree(t);
> }
>
> -void psi_trigger_replace(void **trigger_ptr, struct psi_trigger *new)
> -{
> - struct psi_trigger *old = *trigger_ptr;
> -
> - if (static_branch_likely(&psi_disabled))
> - return;
> -
> - rcu_assign_pointer(*trigger_ptr, new);
> - if (old)
> - kref_put(&old->refcount, psi_trigger_destroy);
> -}
> -
> __poll_t psi_trigger_poll(void **trigger_ptr,
> struct file *file, poll_table *wait)
> {
> @@ -1275,24 +1266,15 @@ __poll_t psi_trigger_poll(void **trigger_ptr,
> if (static_branch_likely(&psi_disabled))
> return DEFAULT_POLLMASK | EPOLLERR | EPOLLPRI;
>
> - rcu_read_lock();
> -
> - t = rcu_dereference(*(void __rcu __force **)trigger_ptr);
> - if (!t) {
> - rcu_read_unlock();
> + t = smp_load_acquire(trigger_ptr);
> + if (!t)
> return DEFAULT_POLLMASK | EPOLLERR | EPOLLPRI;
> - }
> - kref_get(&t->refcount);
> -
> - rcu_read_unlock();
>
> poll_wait(file, &t->event_wait, wait);
>
> if (cmpxchg(&t->event, 1, 0) == 1)
> ret |= EPOLLPRI;
>
> - kref_put(&t->refcount, psi_trigger_destroy);
> -
> return ret;
> }
>
> @@ -1316,14 +1298,24 @@ static ssize_t psi_write(struct file *file, const char __user *user_buf,
>
> buf[buf_size - 1] = '\0';
>
> - new = psi_trigger_create(&psi_system, buf, nbytes, res);
> - if (IS_ERR(new))
> - return PTR_ERR(new);
> -
> seq = file->private_data;
> +
> /* Take seq->lock to protect seq->private from concurrent writes */
> mutex_lock(&seq->lock);
> - psi_trigger_replace(&seq->private, new);
> +
> + /* Allow only one trigger per file descriptor */
> + if (seq->private) {
> + mutex_unlock(&seq->lock);
> + return -EBUSY;
> + }
> +
> + new = psi_trigger_create(&psi_system, buf, nbytes, res);
> + if (IS_ERR(new)) {
> + mutex_unlock(&seq->lock);
> + return PTR_ERR(new);
> + }
> +
> + smp_store_release(&seq->private, new);
> mutex_unlock(&seq->lock);
>
> return nbytes;
> @@ -1358,7 +1350,7 @@ static int psi_fop_release(struct inode *inode, struct file *file)
> {
> struct seq_file *seq = file->private_data;
>
> - psi_trigger_replace(&seq->private, NULL);
> + psi_trigger_destroy(seq->private);
> return single_release(inode, file);
> }
>
>
