From: Suren Baghdasaryan <surenb@google.com> To: Johannes Weiner <hannes@cmpxchg.org> Cc: Linus Torvalds <torvalds@linux-foundation.org>, Eric Biggers <ebiggers@kernel.org>, Tejun Heo <tj@kernel.org>, Zefan Li <lizefan.x@bytedance.com>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, Steven Rostedt <rostedt@goodmis.org>, Benjamin Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, Daniel Bristot de Oliveira <bristot@redhat.com>, Jonathan Corbet <corbet@lwn.net>, "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, cgroups mailinglist <cgroups@vger.kernel.org>, stable <stable@vger.kernel.org>, kernel-team <kernel-team@android.com>, syzbot <syzbot+cdb5dd11c97cc532efad@syzkaller.appspotmail.com> Subject: Re: [PATCH v3 1/1] psi: Fix uaf issue when psi trigger is destroyed while being polled Date: Wed, 12 Jan 2022 09:43:43 -0800 [thread overview] Message-ID: <CAJuCfpGHLXDvMU1GLMcgK_K72_ErPhbcFh1ZvEeHg025yinNuw@mail.gmail.com> (raw) In-Reply-To: <Yd7oPlxCpnzNmFzc@cmpxchg.org> ) On Wed, Jan 12, 2022 at 6:40 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Tue, Jan 11, 2022 at 03:23:09PM -0800, Suren Baghdasaryan wrote: > > 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") > > Cc: stable@vger.kernel.org > > Reported-by: syzbot+cdb5dd11c97cc532efad@syzkaller.appspotmail.com > > Analyzed-by: Eric Biggers <ebiggers@kernel.org> > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> Hmm. kernel test robot notified me of new (which are not really new) warnings but I don't think this patch specifically introduced them: kernel/sched/psi.c:1112:21: warning: no previous prototype for function 'psi_trigger_create' [-Wmissing-prototypes] struct psi_trigger *psi_trigger_create(struct psi_group *group, ^ kernel/sched/psi.c:1112:1: note: declare 'static' if the function is not intended to be used outside of this translation unit struct psi_trigger *psi_trigger_create(struct psi_group *group, ^ static >> kernel/sched/psi.c:1182:6: warning: no previous prototype for function 'psi_trigger_destroy' [-Wmissing-prototypes] void psi_trigger_destroy(struct psi_trigger *t) ^ kernel/sched/psi.c:1182:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void psi_trigger_destroy(struct psi_trigger *t) ^ static kernel/sched/psi.c:1249:10: warning: no previous prototype for function 'psi_trigger_poll' [-Wmissing-prototypes] __poll_t psi_trigger_poll(void **trigger_ptr, ^ kernel/sched/psi.c:1249:1: note: declare 'static' if the function is not intended to be used outside of this translation unit __poll_t psi_trigger_poll(void **trigger_ptr, ^ This happens with the following config: CONFIG_CGROUPS=n CONFIG_PSI=y With cgroups disabled these functions are defined as non-static but are not defined in the header (https://elixir.bootlin.com/linux/latest/source/include/linux/psi.h#L28) since the only external user cgroup.c is disabled. The cleanest way to fix these I think is by doing smth like this in psi.c: struct psi_trigger *_psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { // original psi_trigger_create code } #ifdef CONFIG_CGROUPS struct psi_trigger *psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { return _psi_trigger_create(group, buf, nbytes, res); } #else static struct psi_trigger *psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { return _psi_trigger_create(group, buf, nbytes, res); } #endif Two questions: 1. Is this even worth fixing? 2. If so, I would like to do that as a separate patch (these warnings are unrelated to the changes in this patch). Would that be ok? Thanks, Suren.
WARNING: multiple messages have this Message-ID (diff)
From: Suren Baghdasaryan <surenb@google.com> To: Johannes Weiner <hannes@cmpxchg.org> Cc: Linus Torvalds <torvalds@linux-foundation.org>, Eric Biggers <ebiggers@kernel.org>, Tejun Heo <tj@kernel.org>, Zefan Li <lizefan.x@bytedance.com>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, Steven Rostedt <rostedt@goodmis.org>, Benjamin Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, Daniel Bristot de Oliveira <bristot@redhat.com>, Jonathan Corbet <corbet@lwn.net>, "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, cgroups mailinglist <cgroups@vger.kernel.org>, stable <stable@vger.kernel.org>, kernel-team <kernel-team@android.com>, syzbot Subject: Re: [PATCH v3 1/1] psi: Fix uaf issue when psi trigger is destroyed while being polled Date: Wed, 12 Jan 2022 09:43:43 -0800 [thread overview] Message-ID: <CAJuCfpGHLXDvMU1GLMcgK_K72_ErPhbcFh1ZvEeHg025yinNuw@mail.gmail.com> (raw) In-Reply-To: <Yd7oPlxCpnzNmFzc@cmpxchg.org> ) On Wed, Jan 12, 2022 at 6:40 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Tue, Jan 11, 2022 at 03:23:09PM -0800, Suren Baghdasaryan wrote: > > 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") > > Cc: stable@vger.kernel.org > > Reported-by: syzbot+cdb5dd11c97cc532efad@syzkaller.appspotmail.com > > Analyzed-by: Eric Biggers <ebiggers@kernel.org> > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> Hmm. kernel test robot notified me of new (which are not really new) warnings but I don't think this patch specifically introduced them: kernel/sched/psi.c:1112:21: warning: no previous prototype for function 'psi_trigger_create' [-Wmissing-prototypes] struct psi_trigger *psi_trigger_create(struct psi_group *group, ^ kernel/sched/psi.c:1112:1: note: declare 'static' if the function is not intended to be used outside of this translation unit struct psi_trigger *psi_trigger_create(struct psi_group *group, ^ static >> kernel/sched/psi.c:1182:6: warning: no previous prototype for function 'psi_trigger_destroy' [-Wmissing-prototypes] void psi_trigger_destroy(struct psi_trigger *t) ^ kernel/sched/psi.c:1182:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void psi_trigger_destroy(struct psi_trigger *t) ^ static kernel/sched/psi.c:1249:10: warning: no previous prototype for function 'psi_trigger_poll' [-Wmissing-prototypes] __poll_t psi_trigger_poll(void **trigger_ptr, ^ kernel/sched/psi.c:1249:1: note: declare 'static' if the function is not intended to be used outside of this translation unit __poll_t psi_trigger_poll(void **trigger_ptr, ^ This happens with the following config: CONFIG_CGROUPS=n CONFIG_PSI=y With cgroups disabled these functions are defined as non-static but are not defined in the header (https://elixir.bootlin.com/linux/latest/source/include/linux/psi.h#L28) since the only external user cgroup.c is disabled. The cleanest way to fix these I think is by doing smth like this in psi.c: struct psi_trigger *_psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { // original psi_trigger_create code } #ifdef CONFIG_CGROUPS struct psi_trigger *psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { return _psi_trigger_create(group, buf, nbytes, res); } #else static struct psi_trigger *psi_trigger_create(struct psi_group *group, char *buf, size_t nbytes, enum psi_res res) { return _psi_trigger_create(group, buf, nbytes, res); } #endif Two questions: 1. Is this even worth fixing? 2. If so, I would like to do that as a separate patch (these warnings are unrelated to the changes in this patch). Would that be ok? Thanks, Suren.
next prev parent reply other threads:[~2022-01-12 17:44 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-11 23:23 [PATCH v3 1/1] psi: Fix uaf issue when psi trigger is destroyed while being polled Suren Baghdasaryan 2022-01-12 6:46 ` Eric Biggers 2022-01-12 10:03 ` Peter Zijlstra 2022-01-12 18:03 ` Linus Torvalds 2022-01-12 18:03 ` Linus Torvalds 2022-01-12 14:39 ` Johannes Weiner 2022-01-12 17:43 ` Suren Baghdasaryan [this message] 2022-01-12 17:43 ` Suren Baghdasaryan 2022-01-12 17:49 ` Suren Baghdasaryan 2022-01-12 17:49 ` Suren Baghdasaryan 2022-01-12 18:16 ` Matthew Wilcox 2022-01-12 18:16 ` Matthew Wilcox 2022-01-12 18:26 ` Suren Baghdasaryan 2022-01-12 18:26 ` Suren Baghdasaryan 2022-01-12 18:44 ` Eric Biggers 2022-01-12 18:44 ` Eric Biggers 2022-01-12 18:53 ` Suren Baghdasaryan 2022-01-12 18:53 ` Suren Baghdasaryan 2022-01-12 19:04 ` Eric Biggers 2022-01-12 19:04 ` Eric Biggers 2022-01-12 19:06 ` Suren Baghdasaryan 2022-01-12 19:06 ` Suren Baghdasaryan 2022-01-12 19:49 ` Suren Baghdasaryan 2022-01-12 19:49 ` Suren Baghdasaryan 2022-01-12 15:18 ` kernel test robot 2022-01-12 16:39 ` kernel test robot 2022-01-12 16:39 ` kernel test robot 2022-01-18 11:18 ` [tip: sched/urgent] " tip-bot2 for Suren Baghdasaryan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAJuCfpGHLXDvMU1GLMcgK_K72_ErPhbcFh1ZvEeHg025yinNuw@mail.gmail.com \ --to=surenb@google.com \ --cc=bristot@redhat.com \ --cc=bsegall@google.com \ --cc=cgroups@vger.kernel.org \ --cc=corbet@lwn.net \ --cc=dietmar.eggemann@arm.com \ --cc=ebiggers@kernel.org \ --cc=hannes@cmpxchg.org \ --cc=juri.lelli@redhat.com \ --cc=kernel-team@android.com \ --cc=linux-doc@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lizefan.x@bytedance.com \ --cc=mgorman@suse.de \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=rostedt@goodmis.org \ --cc=stable@vger.kernel.org \ --cc=syzbot+cdb5dd11c97cc532efad@syzkaller.appspotmail.com \ --cc=tj@kernel.org \ --cc=torvalds@linux-foundation.org \ --cc=vincent.guittot@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.