* [PATCH v1 00/20] signal: refactor some functions
@ 2018-06-01 13:22 Christian Brauner
2018-06-01 13:22 ` [PATCH v2 01/17] signal: make force_sigsegv() void Christian Brauner
` (16 more replies)
0 siblings, 17 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
Hey,
This series refactors a bunch of functions in signal.c to simplify parts
of the code.
As requested, v2 drops all 3 commits that were pure coding-style
changes. The only other coding-style change is the simplification of
rt_sigaction which seemed to be worth keeping.
The greatest single change is declaring the static do_sigpending()
helper as void which makes it possible to remove a bunch of unnecessary
checks in the syscalls later on.
Thanks!
Christian
Christian Brauner (17):
signal: make force_sigsegv() void
signal: make kill_as_cred_perm() return bool
signal: make may_ptrace_stop() return bool
signal: make do_sigpending() void
signal: simplify rt_sigaction()
signal: make kill_ok_by_cred() return bool
signal: make sig_handler_ignored() return bool
signal: make sig_task_ignored() return bool
signal: make sig_ignored() return bool
signal: make has_pending_signals() return bool
signal: make recalc_sigpending_tsk() return bool
signal: make unhandled_signal() return bool
signal: make flush_sigqueue_mask() void
signal: make wants_signal() return bool
signal: make legacy_queue() return bool
signal: make security_task_kill() return bool
signal: make get_signal() return bool
include/linux/sched/signal.h | 2 +-
include/linux/signal.h | 4 +-
kernel/signal.c | 200 +++++++++++++++++------------------
3 files changed, 103 insertions(+), 103 deletions(-)
--
2.17.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 01/17] signal: make force_sigsegv() void
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 02/17] signal: make kill_as_cred_perm() return bool Christian Brauner
` (15 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
force_sigsegv() returned 0 unconditionally so it doesn't make sense to have
it return at all. In addition, there are no callers that check
force_sigsegv()'s return value.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* unchanged
---
include/linux/sched/signal.h | 2 +-
kernel/signal.c | 7 ++-----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 113d1ad1ced7..e138ac16c650 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -314,7 +314,7 @@ int force_sig_pkuerr(void __user *addr, u32 pkey);
int force_sig_ptrace_errno_trap(int errno, void __user *addr);
extern int send_sig_info(int, struct siginfo *, struct task_struct *);
-extern int force_sigsegv(int, struct task_struct *);
+extern void force_sigsegv(int sig, struct task_struct *p);
extern int force_sig_info(int, struct siginfo *, struct task_struct *);
extern int __kill_pgrp_info(int sig, struct siginfo *info, struct pid *pgrp);
extern int kill_pid_info(int sig, struct siginfo *info, struct pid *pid);
diff --git a/kernel/signal.c b/kernel/signal.c
index 9c33163a6165..c756008d589e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1468,8 +1468,7 @@ send_sig(int sig, struct task_struct *p, int priv)
return send_sig_info(sig, __si_special(priv), p);
}
-void
-force_sig(int sig, struct task_struct *p)
+void force_sig(int sig, struct task_struct *p)
{
force_sig_info(sig, SEND_SIG_PRIV, p);
}
@@ -1480,8 +1479,7 @@ force_sig(int sig, struct task_struct *p)
* the problem was already a SIGSEGV, we'll want to
* make sure we don't even try to deliver the signal..
*/
-int
-force_sigsegv(int sig, struct task_struct *p)
+void force_sigsegv(int sig, struct task_struct *p)
{
if (sig == SIGSEGV) {
unsigned long flags;
@@ -1490,7 +1488,6 @@ force_sigsegv(int sig, struct task_struct *p)
spin_unlock_irqrestore(&p->sighand->siglock, flags);
}
force_sig(SIGSEGV, p);
- return 0;
}
int force_sig_fault(int sig, int code, void __user *addr
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 02/17] signal: make kill_as_cred_perm() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
2018-06-01 13:22 ` [PATCH v2 01/17] signal: make force_sigsegv() void Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 03/17] signal: make may_ptrace_stop() " Christian Brauner
` (14 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
kill_as_cred_perm() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* simplify to use a single return
---
kernel/signal.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index c756008d589e..6c71d6b8d2b8 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1349,14 +1349,15 @@ static int kill_proc_info(int sig, struct siginfo *info, pid_t pid)
return error;
}
-static int kill_as_cred_perm(const struct cred *cred,
- struct task_struct *target)
+static inline bool kill_as_cred_perm(const struct cred *cred,
+ struct task_struct *target)
{
const struct cred *pcred = __task_cred(target);
- if (!uid_eq(cred->euid, pcred->suid) && !uid_eq(cred->euid, pcred->uid) &&
- !uid_eq(cred->uid, pcred->suid) && !uid_eq(cred->uid, pcred->uid))
- return 0;
- return 1;
+
+ return uid_eq(cred->euid, pcred->suid) ||
+ uid_eq(cred->euid, pcred->uid) ||
+ uid_eq(cred->uid, pcred->suid) ||
+ uid_eq(cred->uid, pcred->uid);
}
/* like kill_pid_info(), but doesn't use uid/euid of "current" */
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 03/17] signal: make may_ptrace_stop() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
2018-06-01 13:22 ` [PATCH v2 01/17] signal: make force_sigsegv() void Christian Brauner
2018-06-01 13:22 ` [PATCH v2 02/17] signal: make kill_as_cred_perm() return bool Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 15:56 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 04/17] signal: make do_sigpending() void Christian Brauner
` (13 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
may_ptrace_stop() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* unchanged
---
kernel/signal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 6c71d6b8d2b8..bc750fb4ddcc 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1889,10 +1889,10 @@ static void do_notify_parent_cldstop(struct task_struct *tsk,
spin_unlock_irqrestore(&sighand->siglock, flags);
}
-static inline int may_ptrace_stop(void)
+static inline bool may_ptrace_stop(void)
{
if (!likely(current->ptrace))
- return 0;
+ return false;
/*
* Are we in the middle of do_coredump?
* If so and our tracer is also part of the coredump stopping
@@ -1906,11 +1906,11 @@ static inline int may_ptrace_stop(void)
* block in TASK_TRACED. But PTRACE_EVENT_EXIT can be reported
* after SIGKILL was already dequeued.
*/
- if (unlikely(current->mm->core_state) &&
- unlikely(current->mm == current->parent->mm))
- return 0;
+ if (likely(!current->mm->core_state ||
+ current->mm != current->parent->mm))
+ return false;
- return 1;
+ return true;
}
/*
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 04/17] signal: make do_sigpending() void
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (2 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 03/17] signal: make may_ptrace_stop() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 16:29 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 05/17] signal: simplify rt_sigaction() Christian Brauner
` (12 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
do_sigpending() returned 0 unconditionally so it doesn't make sense to have
it return at all. This allows us to simplify a bunch of syscall callers.
Signed-off-by: Christian Brauner <christian@brauner.io>
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
---
v1->v2:
* unchanged
v0->v1:
* unchanged
---
kernel/signal.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index bc750fb4ddcc..7c770556b085 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2767,7 +2767,7 @@ COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, int, how, compat_sigset_t __user *, nset,
}
#endif
-static int do_sigpending(sigset_t *set)
+static void do_sigpending(sigset_t *set)
{
spin_lock_irq(¤t->sighand->siglock);
sigorsets(set, ¤t->pending.signal,
@@ -2776,7 +2776,6 @@ static int do_sigpending(sigset_t *set)
/* Outside the lock because only this thread touches it. */
sigandsets(set, ¤t->blocked, set);
- return 0;
}
/**
@@ -2788,15 +2787,16 @@ static int do_sigpending(sigset_t *set)
SYSCALL_DEFINE2(rt_sigpending, sigset_t __user *, uset, size_t, sigsetsize)
{
sigset_t set;
- int err;
if (sigsetsize > sizeof(*uset))
return -EINVAL;
- err = do_sigpending(&set);
- if (!err && copy_to_user(uset, &set, sigsetsize))
- err = -EFAULT;
- return err;
+ do_sigpending(&set);
+
+ if (copy_to_user(uset, &set, sigsetsize))
+ return -EFAULT;
+
+ return 0;
}
#ifdef CONFIG_COMPAT
@@ -2804,15 +2804,13 @@ COMPAT_SYSCALL_DEFINE2(rt_sigpending, compat_sigset_t __user *, uset,
compat_size_t, sigsetsize)
{
sigset_t set;
- int err;
if (sigsetsize > sizeof(*uset))
return -EINVAL;
- err = do_sigpending(&set);
- if (!err)
- err = put_compat_sigset(uset, &set, sigsetsize);
- return err;
+ do_sigpending(&set);
+
+ return put_compat_sigset(uset, &set, sigsetsize);
}
#endif
@@ -3647,25 +3645,26 @@ int __compat_save_altstack(compat_stack_t __user *uss, unsigned long sp)
SYSCALL_DEFINE1(sigpending, old_sigset_t __user *, uset)
{
sigset_t set;
- int err;
if (sizeof(old_sigset_t) > sizeof(*uset))
return -EINVAL;
- err = do_sigpending(&set);
- if (!err && copy_to_user(uset, &set, sizeof(old_sigset_t)))
- err = -EFAULT;
- return err;
+ do_sigpending(&set);
+
+ if (copy_to_user(uset, &set, sizeof(old_sigset_t)))
+ return -EFAULT;
+
+ return 0;
}
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE1(sigpending, compat_old_sigset_t __user *, set32)
{
sigset_t set;
- int err = do_sigpending(&set);
- if (!err)
- err = put_user(set.sig[0], set32);
- return err;
+
+ do_sigpending(&set);
+
+ return put_user(set.sig[0], set32);
}
#endif
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 05/17] signal: simplify rt_sigaction()
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (3 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 04/17] signal: make do_sigpending() void Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 16:04 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 06/17] signal: make kill_ok_by_cred() return bool Christian Brauner
` (11 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
The goto is not needed and does not add any clarity. Simply return -EINVAL
on unexpected sigset_t struct size directly.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* [Christoph Hellwig] additional cleanups
v0->v1:
* unchanged
---
kernel/signal.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 7c770556b085..00ed69d33282 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3735,25 +3735,23 @@ SYSCALL_DEFINE4(rt_sigaction, int, sig,
size_t, sigsetsize)
{
struct k_sigaction new_sa, old_sa;
- int ret = -EINVAL;
+ int ret;
/* XXX: Don't preclude handling different sized sigset_t's. */
if (sigsetsize != sizeof(sigset_t))
- goto out;
+ return -EINVAL;
- if (act) {
- if (copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)))
- return -EFAULT;
- }
+ if (act && copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)))
+ return -EFAULT;
ret = do_sigaction(sig, act ? &new_sa : NULL, oact ? &old_sa : NULL);
+ if (ret)
+ return ret;
- if (!ret && oact) {
- if (copy_to_user(oact, &old_sa.sa, sizeof(old_sa.sa)))
- return -EFAULT;
- }
-out:
- return ret;
+ if (oact && copy_to_user(oact, &old_sa.sa, sizeof(old_sa.sa)))
+ return -EFAULT;
+
+ return 0;
}
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE4(rt_sigaction, int, sig,
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 06/17] signal: make kill_ok_by_cred() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (4 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 05/17] signal: simplify rt_sigaction() Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 07/17] signal: make sig_handler_ignored() " Christian Brauner
` (10 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
kill_ok_by_cred() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* unchanged
---
kernel/signal.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 00ed69d33282..bbd15e984a84 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -717,21 +717,16 @@ static inline bool si_fromuser(const struct siginfo *info)
/*
* called with RCU read lock from check_kill_permission()
*/
-static int kill_ok_by_cred(struct task_struct *t)
+static bool kill_ok_by_cred(struct task_struct *t)
{
const struct cred *cred = current_cred();
const struct cred *tcred = __task_cred(t);
- if (uid_eq(cred->euid, tcred->suid) ||
- uid_eq(cred->euid, tcred->uid) ||
- uid_eq(cred->uid, tcred->suid) ||
- uid_eq(cred->uid, tcred->uid))
- return 1;
-
- if (ns_capable(tcred->user_ns, CAP_KILL))
- return 1;
-
- return 0;
+ return uid_eq(cred->euid, tcred->suid) ||
+ uid_eq(cred->euid, tcred->uid) ||
+ uid_eq(cred->uid, tcred->suid) ||
+ uid_eq(cred->uid, tcred->uid) ||
+ ns_capable(tcred->user_ns, CAP_KILL);
}
/*
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 07/17] signal: make sig_handler_ignored() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (5 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 06/17] signal: make kill_ok_by_cred() return bool Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 08/17] signal: make sig_task_ignored() " Christian Brauner
` (9 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
sig_handler_ignored() already behaves like a boolean function. Let's
actually declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index bbd15e984a84..62e2586f961e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -65,11 +65,11 @@ static void __user *sig_handler(struct task_struct *t, int sig)
return t->sighand->action[sig - 1].sa.sa_handler;
}
-static int sig_handler_ignored(void __user *handler, int sig)
+static inline bool sig_handler_ignored(void __user *handler, int sig)
{
/* Is it explicitly or implicitly ignored? */
return handler == SIG_IGN ||
- (handler == SIG_DFL && sig_kernel_ignore(sig));
+ (handler == SIG_DFL && sig_kernel_ignore(sig));
}
static int sig_task_ignored(struct task_struct *t, int sig, bool force)
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 08/17] signal: make sig_task_ignored() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (6 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 07/17] signal: make sig_handler_ignored() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 09/17] signal: make sig_ignored() " Christian Brauner
` (8 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
sig_task_ignored() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 62e2586f961e..208e99b57e6f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -72,7 +72,7 @@ static inline bool sig_handler_ignored(void __user *handler, int sig)
(handler == SIG_DFL && sig_kernel_ignore(sig));
}
-static int sig_task_ignored(struct task_struct *t, int sig, bool force)
+static bool sig_task_ignored(struct task_struct *t, int sig, bool force)
{
void __user *handler;
@@ -80,7 +80,7 @@ static int sig_task_ignored(struct task_struct *t, int sig, bool force)
if (unlikely(t->signal->flags & SIGNAL_UNKILLABLE) &&
handler == SIG_DFL && !(force && sig_kernel_only(sig)))
- return 1;
+ return true;
return sig_handler_ignored(handler, sig);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 09/17] signal: make sig_ignored() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (7 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 08/17] signal: make sig_task_ignored() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 10/17] signal: make has_pending_signals() " Christian Brauner
` (7 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
sig_ignored() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 208e99b57e6f..e48675bd3be8 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -85,7 +85,7 @@ static bool sig_task_ignored(struct task_struct *t, int sig, bool force)
return sig_handler_ignored(handler, sig);
}
-static int sig_ignored(struct task_struct *t, int sig, bool force)
+static bool sig_ignored(struct task_struct *t, int sig, bool force)
{
/*
* Blocked signals are never ignored, since the
@@ -93,7 +93,7 @@ static int sig_ignored(struct task_struct *t, int sig, bool force)
* unblocked.
*/
if (sigismember(&t->blocked, sig) || sigismember(&t->real_blocked, sig))
- return 0;
+ return false;
/*
* Tracers may want to know about even ignored signal unless it
@@ -101,7 +101,7 @@ static int sig_ignored(struct task_struct *t, int sig, bool force)
* by SIGNAL_UNKILLABLE task.
*/
if (t->ptrace && sig != SIGKILL)
- return 0;
+ return false;
return sig_task_ignored(t, sig, force);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 10/17] signal: make has_pending_signals() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (8 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 09/17] signal: make sig_ignored() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 16:16 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 11/17] signal: make recalc_sigpending_tsk() " Christian Brauner
` (6 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
has_pending_signals() already behaves like a boolean function. Let's
actually declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index e48675bd3be8..faf38b46d124 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -110,30 +110,33 @@ static bool sig_ignored(struct task_struct *t, int sig, bool force)
* Re-calculate pending state from the set of locally pending
* signals, globally pending signals, and blocked signals.
*/
-static inline int has_pending_signals(sigset_t *signal, sigset_t *blocked)
+static inline bool has_pending_signals(sigset_t *signal, sigset_t *blocked)
{
unsigned long ready;
long i;
switch (_NSIG_WORDS) {
default:
- for (i = _NSIG_WORDS, ready = 0; --i >= 0 ;)
- ready |= signal->sig[i] &~ blocked->sig[i];
+ for (i = _NSIG_WORDS, ready = 0; --i >= 0;)
+ ready |= signal->sig[i] & ~blocked->sig[i];
break;
- case 4: ready = signal->sig[3] &~ blocked->sig[3];
- ready |= signal->sig[2] &~ blocked->sig[2];
- ready |= signal->sig[1] &~ blocked->sig[1];
- ready |= signal->sig[0] &~ blocked->sig[0];
+ case 4:
+ ready = signal->sig[3] & ~blocked->sig[3];
+ ready |= signal->sig[2] & ~blocked->sig[2];
+ ready |= signal->sig[1] & ~blocked->sig[1];
+ ready |= signal->sig[0] & ~blocked->sig[0];
break;
- case 2: ready = signal->sig[1] &~ blocked->sig[1];
- ready |= signal->sig[0] &~ blocked->sig[0];
+ case 2:
+ ready = signal->sig[1] & ~blocked->sig[1];
+ ready |= signal->sig[0] & ~blocked->sig[0];
break;
- case 1: ready = signal->sig[0] &~ blocked->sig[0];
+ case 1:
+ ready = signal->sig[0] & ~blocked->sig[0];
}
- return ready != 0;
+ return ready != 0;
}
#define PENDING(p,b) has_pending_signals(&(p)->signal, (b))
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 11/17] signal: make recalc_sigpending_tsk() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (9 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 10/17] signal: make has_pending_signals() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 12/17] signal: make unhandled_signal() " Christian Brauner
` (5 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
recalc_sigpending_tsk() already behaves like a boolean function. Let's
actually declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index faf38b46d124..d5a3b1ef775e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -141,20 +141,21 @@ static inline bool has_pending_signals(sigset_t *signal, sigset_t *blocked)
#define PENDING(p,b) has_pending_signals(&(p)->signal, (b))
-static int recalc_sigpending_tsk(struct task_struct *t)
+static bool recalc_sigpending_tsk(struct task_struct *t)
{
if ((t->jobctl & JOBCTL_PENDING_MASK) ||
PENDING(&t->pending, &t->blocked) ||
PENDING(&t->signal->shared_pending, &t->blocked)) {
set_tsk_thread_flag(t, TIF_SIGPENDING);
- return 1;
+ return true;
}
+
/*
* We must never clear the flag in another thread, or in current
* when it's possible the current syscall is returning -ERESTART*.
* So we don't clear it here, and only callers who know they should do.
*/
- return 0;
+ return false;
}
/*
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 12/17] signal: make unhandled_signal() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (10 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 11/17] signal: make recalc_sigpending_tsk() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 13/17] signal: make flush_sigqueue_mask() void Christian Brauner
` (4 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
unhandled_signal() already behaves like a boolean function. Let's actually
declare it as such too.
All callers treat it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
include/linux/signal.h | 2 +-
kernel/signal.c | 11 +++++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index a9bc7e1b077e..1145d7061ed9 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -284,7 +284,7 @@ static inline void disallow_signal(int sig)
extern struct kmem_cache *sighand_cachep;
-int unhandled_signal(struct task_struct *tsk, int sig);
+extern bool unhandled_signal(struct task_struct *tsk, int sig);
/*
* In POSIX a signal is sent either to a specific thread (Linux task)
diff --git a/kernel/signal.c b/kernel/signal.c
index d5a3b1ef775e..a41af5a7749f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -508,13 +508,16 @@ flush_signal_handlers(struct task_struct *t, int force_default)
}
}
-int unhandled_signal(struct task_struct *tsk, int sig)
+bool unhandled_signal(struct task_struct *tsk, int sig)
{
- void __user *handler = tsk->sighand->action[sig-1].sa.sa_handler;
+ void __user *handler = tsk->sighand->action[sig - 1].sa.sa_handler;
+
if (is_global_init(tsk))
- return 1;
+ return true;
+
if (handler != SIG_IGN && handler != SIG_DFL)
- return 0;
+ return false;
+
/* if ptraced, let the tracer determine */
return !tsk->ptrace;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 13/17] signal: make flush_sigqueue_mask() void
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (11 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 12/17] signal: make unhandled_signal() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 14/17] signal: make wants_signal() return bool Christian Brauner
` (3 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
The return value of flush_sigqueue_mask() is never checked anywhere.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index a41af5a7749f..dd32a1ce58e2 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -691,14 +691,14 @@ void signal_wake_up_state(struct task_struct *t, unsigned int state)
*
* All callers must be holding the siglock.
*/
-static int flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
+static void flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
{
struct sigqueue *q, *n;
sigset_t m;
sigandsets(&m, mask, &s->signal);
if (sigisemptyset(&m))
- return 0;
+ return;
sigandnsets(&s->signal, &s->signal, mask);
list_for_each_entry_safe(q, n, &s->list, list) {
@@ -707,7 +707,6 @@ static int flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
__sigqueue_free(q);
}
}
- return 1;
}
static inline int is_si_special(const struct siginfo *info)
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 14/17] signal: make wants_signal() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (12 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 13/17] signal: make flush_sigqueue_mask() void Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 15/17] signal: make legacy_queue() " Christian Brauner
` (2 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
wants_signal() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index dd32a1ce58e2..8ee3e4a57117 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -883,16 +883,20 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
* as soon as they're available, so putting the signal on the shared queue
* will be equivalent to sending it to one such thread.
*/
-static inline int wants_signal(int sig, struct task_struct *p)
+static inline bool wants_signal(int sig, struct task_struct *p)
{
if (sigismember(&p->blocked, sig))
- return 0;
+ return false;
+
if (p->flags & PF_EXITING)
- return 0;
+ return false;
+
if (sig == SIGKILL)
- return 1;
+ return true;
+
if (task_is_stopped_or_traced(p))
- return 0;
+ return false;
+
return task_curr(p) || !signal_pending(p);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 15/17] signal: make legacy_queue() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (13 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 14/17] signal: make wants_signal() return bool Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 16/17] signal: make security_task_kill() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 17/17] signal: make get_signal() " Christian Brauner
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
legacy_queue() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 8ee3e4a57117..515fa59a0e9c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -976,7 +976,7 @@ static void complete_signal(int sig, struct task_struct *p, int group)
return;
}
-static inline int legacy_queue(struct sigpending *signals, int sig)
+static inline bool legacy_queue(struct sigpending *signals, int sig)
{
return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 16/17] signal: make security_task_kill() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (14 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 15/17] signal: make legacy_queue() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
2018-06-01 16:26 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 17/17] signal: make get_signal() " Christian Brauner
16 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
security_task_kill() already behaves like a boolean function. Let's
actually declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
kernel/signal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 515fa59a0e9c..d5f9472a0935 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1922,10 +1922,10 @@ static inline bool may_ptrace_stop(void)
* Return non-zero if there is a SIGKILL that should be waking us up.
* Called with the siglock held.
*/
-static int sigkill_pending(struct task_struct *tsk)
+static bool sigkill_pending(struct task_struct *tsk)
{
- return sigismember(&tsk->pending.signal, SIGKILL) ||
- sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
+ return sigismember(&tsk->pending.signal, SIGKILL) ||
+ sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
}
/*
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 17/17] signal: make get_signal() return bool
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
` (15 preceding siblings ...)
2018-06-01 13:22 ` [PATCH v2 16/17] signal: make security_task_kill() " Christian Brauner
@ 2018-06-01 13:22 ` Christian Brauner
16 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 13:22 UTC (permalink / raw)
To: linux-kernel
Cc: ebiederm, gregkh, mingo, james.morris, keescook, peterz, sds,
viro, akpm, oleg, Christian Brauner
make get_signal() already behaves like a boolean function. Let's actually
declare it as such too.
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v1->v2:
* unchanged
v0->v1:
* patch added
---
include/linux/signal.h | 2 +-
kernel/signal.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 1145d7061ed9..97d5ff809716 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -262,7 +262,7 @@ extern void set_current_blocked(sigset_t *);
extern void __set_current_blocked(const sigset_t *);
extern int show_unhandled_signals;
-extern int get_signal(struct ksignal *ksig);
+extern bool get_signal(struct ksignal *ksig);
extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
extern void exit_signals(struct task_struct *tsk);
extern void kernel_sigaction(int, __sighandler_t);
diff --git a/kernel/signal.c b/kernel/signal.c
index d5f9472a0935..be5b85567ca4 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2305,7 +2305,7 @@ static int ptrace_signal(int signr, siginfo_t *info)
return signr;
}
-int get_signal(struct ksignal *ksig)
+bool get_signal(struct ksignal *ksig)
{
struct sighand_struct *sighand = current->sighand;
struct signal_struct *signal = current->signal;
@@ -2315,7 +2315,7 @@ int get_signal(struct ksignal *ksig)
task_work_run();
if (unlikely(uprobe_deny_signal()))
- return 0;
+ return false;
/*
* Do this once, we can't return to user-mode if freezing() == T.
--
2.17.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 03/17] signal: make may_ptrace_stop() return bool
2018-06-01 13:22 ` [PATCH v2 03/17] signal: make may_ptrace_stop() " Christian Brauner
@ 2018-06-01 15:56 ` Oleg Nesterov
2018-06-01 16:00 ` Christian Brauner
0 siblings, 1 reply; 27+ messages in thread
From: Oleg Nesterov @ 2018-06-01 15:56 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On 06/01, Christian Brauner wrote:
>
> may_ptrace_stop() already behaves like a boolean function. Let's actually
> declare it as such too.
OK. Then probably this patch should only make it return bool and do nothing else?
> - if (unlikely(current->mm->core_state) &&
> - unlikely(current->mm == current->parent->mm))
> - return 0;
> + if (likely(!current->mm->core_state ||
> + current->mm != current->parent->mm))
> + return false;
this is wrong.
Oleg.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 03/17] signal: make may_ptrace_stop() return bool
2018-06-01 15:56 ` Oleg Nesterov
@ 2018-06-01 16:00 ` Christian Brauner
0 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 16:00 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On Fri, Jun 01, 2018 at 05:56:12PM +0200, Oleg Nesterov wrote:
> On 06/01, Christian Brauner wrote:
> >
> > may_ptrace_stop() already behaves like a boolean function. Let's actually
> > declare it as such too.
>
> OK. Then probably this patch should only make it return bool and do nothing else?
>
> > - if (unlikely(current->mm->core_state) &&
> > - unlikely(current->mm == current->parent->mm))
> > - return 0;
> > + if (likely(!current->mm->core_state ||
> > + current->mm != current->parent->mm))
> > + return false;
>
> this is wrong.
This was a suggestion by Al. I just took it over and may have messed it
up while doing so. I'll remove this.
Christian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 05/17] signal: simplify rt_sigaction()
2018-06-01 13:22 ` [PATCH v2 05/17] signal: simplify rt_sigaction() Christian Brauner
@ 2018-06-01 16:04 ` Oleg Nesterov
0 siblings, 0 replies; 27+ messages in thread
From: Oleg Nesterov @ 2018-06-01 16:04 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On 06/01, Christian Brauner wrote:
>
> The goto is not needed and does not add any clarity. Simply return -EINVAL
> on unexpected sigset_t struct size directly.
Agreed, sys_rt_sigaction() looks just ugly and mixes goto/return on failure,
ACK
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 10/17] signal: make has_pending_signals() return bool
2018-06-01 13:22 ` [PATCH v2 10/17] signal: make has_pending_signals() " Christian Brauner
@ 2018-06-01 16:16 ` Oleg Nesterov
2018-06-01 17:45 ` Christian Brauner
0 siblings, 1 reply; 27+ messages in thread
From: Oleg Nesterov @ 2018-06-01 16:16 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On 06/01, Christian Brauner wrote:
>
> has_pending_signals() already behaves like a boolean function. Let's
> actually declare it as such too.
But this patch does more.
> - case 4: ready = signal->sig[3] &~ blocked->sig[3];
> - ready |= signal->sig[2] &~ blocked->sig[2];
> - ready |= signal->sig[1] &~ blocked->sig[1];
> - ready |= signal->sig[0] &~ blocked->sig[0];
> + case 4:
> + ready = signal->sig[3] & ~blocked->sig[3];
> + ready |= signal->sig[2] & ~blocked->sig[2];
> + ready |= signal->sig[1] & ~blocked->sig[1];
> + ready |= signal->sig[0] & ~blocked->sig[0];
> break;
Again, personally I do not care at all. But why do you think the code looks
better after re-formatting? This is subjective, but to me it does not.
In particular, note the extra space before "=" removed by this patch. I guess
it was added on purpose, and to me
ready = signal->sig[3] &~ blocked->sig[3];
ready |= signal->sig[2] &~ blocked->sig[2];
actually looks better thab
ready = signal->sig[3] &~ blocked->sig[3];
ready |= signal->sig[2] &~ blocked->sig[2];
after your patch.
Oleg.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 16/17] signal: make security_task_kill() return bool
2018-06-01 13:22 ` [PATCH v2 16/17] signal: make security_task_kill() " Christian Brauner
@ 2018-06-01 16:26 ` Oleg Nesterov
2018-06-01 17:45 ` Christian Brauner
2018-06-02 9:31 ` Christian Brauner
0 siblings, 2 replies; 27+ messages in thread
From: Oleg Nesterov @ 2018-06-01 16:26 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On 06/01, Christian Brauner wrote:
>
> security_task_kill() already behaves like a boolean function. Let's
> actually declare it as such too.
The subject/changelog is wrong, this patch changes sigkill_pending()
> Signed-off-by: Christian Brauner <christian@brauner.io>
> ---
> v1->v2:
> * unchanged
> v0->v1:
> * patch added
> ---
> kernel/signal.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 515fa59a0e9c..d5f9472a0935 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1922,10 +1922,10 @@ static inline bool may_ptrace_stop(void)
> * Return non-zero if there is a SIGKILL that should be waking us up.
> * Called with the siglock held.
> */
> -static int sigkill_pending(struct task_struct *tsk)
> +static bool sigkill_pending(struct task_struct *tsk)
> {
> - return sigismember(&tsk->pending.signal, SIGKILL) ||
> - sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> + return sigismember(&tsk->pending.signal, SIGKILL) ||
> + sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> }
>
> /*
> --
> 2.17.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 04/17] signal: make do_sigpending() void
2018-06-01 13:22 ` [PATCH v2 04/17] signal: make do_sigpending() void Christian Brauner
@ 2018-06-01 16:29 ` Oleg Nesterov
0 siblings, 0 replies; 27+ messages in thread
From: Oleg Nesterov @ 2018-06-01 16:29 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On 06/01, Christian Brauner wrote:
>
> do_sigpending() returned 0 unconditionally so it doesn't make sense to have
> it return at all. This allows us to simplify a bunch of syscall callers.
Agreed, ACK
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 16/17] signal: make security_task_kill() return bool
2018-06-01 16:26 ` Oleg Nesterov
@ 2018-06-01 17:45 ` Christian Brauner
2018-06-02 9:31 ` Christian Brauner
1 sibling, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 17:45 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On Fri, Jun 01, 2018 at 06:26:25PM +0200, Oleg Nesterov wrote:
> On 06/01, Christian Brauner wrote:
> >
> > security_task_kill() already behaves like a boolean function. Let's
> > actually declare it as such too.
>
> The subject/changelog is wrong, this patch changes sigkill_pending()
I'll add a note in the commit message that superfuous whitespace is
removed too.
Thanks!
Christian
>
> > Signed-off-by: Christian Brauner <christian@brauner.io>
> > ---
> > v1->v2:
> > * unchanged
> > v0->v1:
> > * patch added
> > ---
> > kernel/signal.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index 515fa59a0e9c..d5f9472a0935 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -1922,10 +1922,10 @@ static inline bool may_ptrace_stop(void)
> > * Return non-zero if there is a SIGKILL that should be waking us up.
> > * Called with the siglock held.
> > */
> > -static int sigkill_pending(struct task_struct *tsk)
> > +static bool sigkill_pending(struct task_struct *tsk)
> > {
> > - return sigismember(&tsk->pending.signal, SIGKILL) ||
> > - sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> > + return sigismember(&tsk->pending.signal, SIGKILL) ||
> > + sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> > }
> >
> > /*
> > --
> > 2.17.0
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 10/17] signal: make has_pending_signals() return bool
2018-06-01 16:16 ` Oleg Nesterov
@ 2018-06-01 17:45 ` Christian Brauner
0 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-01 17:45 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On Fri, Jun 01, 2018 at 06:16:58PM +0200, Oleg Nesterov wrote:
> On 06/01, Christian Brauner wrote:
> >
> > has_pending_signals() already behaves like a boolean function. Let's
> > actually declare it as such too.
>
> But this patch does more.
>
> > - case 4: ready = signal->sig[3] &~ blocked->sig[3];
> > - ready |= signal->sig[2] &~ blocked->sig[2];
> > - ready |= signal->sig[1] &~ blocked->sig[1];
> > - ready |= signal->sig[0] &~ blocked->sig[0];
> > + case 4:
> > + ready = signal->sig[3] & ~blocked->sig[3];
> > + ready |= signal->sig[2] & ~blocked->sig[2];
> > + ready |= signal->sig[1] & ~blocked->sig[1];
> > + ready |= signal->sig[0] & ~blocked->sig[0];
> > break;
>
> Again, personally I do not care at all. But why do you think the code looks
> better after re-formatting? This is subjective, but to me it does not.
>
> In particular, note the extra space before "=" removed by this patch. I guess
> it was added on purpose, and to me
>
> ready = signal->sig[3] &~ blocked->sig[3];
> ready |= signal->sig[2] &~ blocked->sig[2];
>
> actually looks better thab
>
> ready = signal->sig[3] &~ blocked->sig[3];
> ready |= signal->sig[2] &~ blocked->sig[2];
>
> after your patch.
I can drop the changes in v3.
Thanks!
Christian
>
> Oleg.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 16/17] signal: make security_task_kill() return bool
2018-06-01 16:26 ` Oleg Nesterov
2018-06-01 17:45 ` Christian Brauner
@ 2018-06-02 9:31 ` Christian Brauner
1 sibling, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2018-06-02 9:31 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, ebiederm, gregkh, mingo, james.morris, keescook,
peterz, sds, viro, akpm
On Fri, Jun 01, 2018 at 06:26:25PM +0200, Oleg Nesterov wrote:
> On 06/01, Christian Brauner wrote:
> >
> > security_task_kill() already behaves like a boolean function. Let's
> > actually declare it as such too.
>
> The subject/changelog is wrong, this patch changes sigkill_pending()
Ah yes, s/security_task_kill()/sigkill_pending()
Thanks!
Christian
>
> > Signed-off-by: Christian Brauner <christian@brauner.io>
> > ---
> > v1->v2:
> > * unchanged
> > v0->v1:
> > * patch added
> > ---
> > kernel/signal.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index 515fa59a0e9c..d5f9472a0935 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -1922,10 +1922,10 @@ static inline bool may_ptrace_stop(void)
> > * Return non-zero if there is a SIGKILL that should be waking us up.
> > * Called with the siglock held.
> > */
> > -static int sigkill_pending(struct task_struct *tsk)
> > +static bool sigkill_pending(struct task_struct *tsk)
> > {
> > - return sigismember(&tsk->pending.signal, SIGKILL) ||
> > - sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> > + return sigismember(&tsk->pending.signal, SIGKILL) ||
> > + sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
> > }
> >
> > /*
> > --
> > 2.17.0
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-06-02 9:32 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-01 13:22 [PATCH v1 00/20] signal: refactor some functions Christian Brauner
2018-06-01 13:22 ` [PATCH v2 01/17] signal: make force_sigsegv() void Christian Brauner
2018-06-01 13:22 ` [PATCH v2 02/17] signal: make kill_as_cred_perm() return bool Christian Brauner
2018-06-01 13:22 ` [PATCH v2 03/17] signal: make may_ptrace_stop() " Christian Brauner
2018-06-01 15:56 ` Oleg Nesterov
2018-06-01 16:00 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 04/17] signal: make do_sigpending() void Christian Brauner
2018-06-01 16:29 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 05/17] signal: simplify rt_sigaction() Christian Brauner
2018-06-01 16:04 ` Oleg Nesterov
2018-06-01 13:22 ` [PATCH v2 06/17] signal: make kill_ok_by_cred() return bool Christian Brauner
2018-06-01 13:22 ` [PATCH v2 07/17] signal: make sig_handler_ignored() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 08/17] signal: make sig_task_ignored() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 09/17] signal: make sig_ignored() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 10/17] signal: make has_pending_signals() " Christian Brauner
2018-06-01 16:16 ` Oleg Nesterov
2018-06-01 17:45 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 11/17] signal: make recalc_sigpending_tsk() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 12/17] signal: make unhandled_signal() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 13/17] signal: make flush_sigqueue_mask() void Christian Brauner
2018-06-01 13:22 ` [PATCH v2 14/17] signal: make wants_signal() return bool Christian Brauner
2018-06-01 13:22 ` [PATCH v2 15/17] signal: make legacy_queue() " Christian Brauner
2018-06-01 13:22 ` [PATCH v2 16/17] signal: make security_task_kill() " Christian Brauner
2018-06-01 16:26 ` Oleg Nesterov
2018-06-01 17:45 ` Christian Brauner
2018-06-02 9:31 ` Christian Brauner
2018-06-01 13:22 ` [PATCH v2 17/17] signal: make get_signal() " Christian Brauner
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.