From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
"Eric W. Biederman" <ebiederm@xmission.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Manfred Spraul <manfred@colorfullife.com>,
Markus Elfring <elfring@users.sourceforge.net>,
Yoji <yoji.fujihar.min@gmail.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH V2] ipc/mqueue.c: change __do_notify() to bypass check_kill_permission()
Date: Tue, 24 Mar 2020 21:09:32 +0100 [thread overview]
Message-ID: <20200324200932.GB24230@redhat.com> (raw)
In-Reply-To: <20200322110901.GA25108@redhat.com>
Commit cc731525f26a ("signal: Remove kernel interal si_code magic")
changed the value of SI_FROMUSER(SI_MESGQ), this means that mq_notify()
no longer works if the sender doesn't have rights to send a signal.
Change __do_notify() to use do_send_sig_info() instead of kill_pid_info()
to avoid check_kill_permission().
This needs the additional notify.sigev_signo != 0 check, shouldn't we
change do_mq_notify() to deny sigev_signo == 0 ?
Test-case:
#include <signal.h>
#include <mqueue.h>
#include <unistd.h>
#include <sys/wait.h>
#include <assert.h>
static int notified;
static void sigh(int sig)
{
notified = 1;
}
int main(void)
{
signal(SIGIO, sigh);
int fd = mq_open("/mq", O_RDWR|O_CREAT, 0666, NULL);
assert(fd >= 0);
struct sigevent se = {
.sigev_notify = SIGEV_SIGNAL,
.sigev_signo = SIGIO,
};
assert(mq_notify(fd, &se) == 0);
if (!fork()) {
assert(setuid(1) == 0);
mq_send(fd, "",1,0);
return 0;
}
wait(NULL);
mq_unlink("/mq");
assert(notified);
return 0;
}
Reported-by: Yoji <yoji.fujihar.min@gmail.com>
Fixes: cc731525f26a ("signal: Remove kernel interal si_code magic")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
ipc/mqueue.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 49a05ba3000d..63b164932ffd 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -774,28 +774,40 @@ static void __do_notify(struct mqueue_inode_info *info)
* synchronously. */
if (info->notify_owner &&
info->attr.mq_curmsgs == 1) {
- struct kernel_siginfo sig_i;
switch (info->notify.sigev_notify) {
case SIGEV_NONE:
break;
- case SIGEV_SIGNAL:
- /* sends signal */
+ case SIGEV_SIGNAL: {
+ struct kernel_siginfo sig_i;
+ struct task_struct *task;
+
+ /* do_mq_notify() accepts sigev_signo == 0, why?? */
+ if (!info->notify.sigev_signo)
+ break;
clear_siginfo(&sig_i);
sig_i.si_signo = info->notify.sigev_signo;
sig_i.si_errno = 0;
sig_i.si_code = SI_MESGQ;
sig_i.si_value = info->notify.sigev_value;
- /* map current pid/uid into info->owner's namespaces */
rcu_read_lock();
+ /* map current pid/uid into info->owner's namespaces */
sig_i.si_pid = task_tgid_nr_ns(current,
ns_of_pid(info->notify_owner));
- sig_i.si_uid = from_kuid_munged(info->notify_user_ns, current_uid());
+ sig_i.si_uid = from_kuid_munged(info->notify_user_ns,
+ current_uid());
+ /*
+ * We can't use kill_pid_info(), this signal should
+ * bypass check_kill_permission(). It is from kernel
+ * but si_fromuser() can't know this.
+ */
+ task = pid_task(info->notify_owner, PIDTYPE_PID);
+ if (task)
+ do_send_sig_info(info->notify.sigev_signo,
+ &sig_i, task, PIDTYPE_TGID);
rcu_read_unlock();
-
- kill_pid_info(info->notify.sigev_signo,
- &sig_i, info->notify_owner);
break;
+ }
case SIGEV_THREAD:
set_cookie(info->notify_cookie, NOTIFY_WOKENUP);
netlink_sendskb(info->notify_sock, info->notify_cookie);
--
2.25.1.362.g51ebf55
next prev parent reply other threads:[~2020-03-24 20:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-22 11:09 [PATCH] ipc/mqueue.c: change __do_notify() to bypass check_kill_permission() Oleg Nesterov
2020-03-22 14:17 ` Eric W. Biederman
2020-03-22 14:59 ` Eric W. Biederman
2020-03-22 20:29 ` Oleg Nesterov
2020-03-23 16:47 ` Eric W. Biederman
2020-03-24 2:12 ` Andrew Morton
2020-03-24 2:57 ` Eric W. Biederman
2020-03-24 11:52 ` Oleg Nesterov
2020-03-24 20:08 ` Oleg Nesterov
2020-03-24 10:35 ` Oleg Nesterov
2020-03-24 20:09 ` Oleg Nesterov [this message]
2020-03-26 12:54 ` [PATCH V2] " Eric W. Biederman
2020-03-27 19:56 ` [PATCH -mm] ipc-mqueuec-change-__do_notify-to-bypass-check_kill_permission-fix Oleg Nesterov
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=20200324200932.GB24230@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dave@stgolabs.net \
--cc=ebiederm@xmission.com \
--cc=elfring@users.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred@colorfullife.com \
--cc=yoji.fujihar.min@gmail.com \
/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: link
Be 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.