On 09/20, Oleg Nesterov wrote:
>
> @@ -908,13 +912,13 @@ long seccomp_get_filter(struct task_struct *task,
> unsigned long filter_off,
> if (!data)
> goto out;
>
> - get_seccomp_filter(task);
> + refcount_inc(&filter->usage);
> spin_unlock_irq(&task->sighand->siglock);
>
> if (copy_to_user(data, fprog->filter, bpf_classic_proglen(fprog)))
> ret = -EFAULT;
>
> - put_seccomp_filter(task);
> + __put_seccomp_filter(filter);
This is the simple fix for -stable, but again, can't we simplify this
code? Afaics we can do get_seccomp_filter() at the start and drop siglock
right after that.
Something like the untested patch (on top of this one) below?
And I can't understand the SECCOMP_MODE_DISABLED check... shouldn't we
simply remove it?
Oleg.
--- x/kernel/seccomp.c
+++ x/kernel/seccomp.c
@@ -858,45 +858,36 @@ long prctl_set_seccomp(unsigned long seccomp_mode, char
__user *filter)
long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
void __user *data)
{
- struct seccomp_filter *filter;
+ struct seccomp_filter *orig, *filter;
struct sock_fprog_kern *fprog;
+ unsigned long count;
long ret;
- unsigned long count = 0;
if (!capable(CAP_SYS_ADMIN) ||
current->seccomp.mode != SECCOMP_MODE_DISABLED) {
return -EACCES;
}
+ if (task->seccomp.mode != SECCOMP_MODE_FILTER)
+ return -EINVAL;
+
spin_lock_irq(&task->sighand->siglock);
- if (task->seccomp.mode != SECCOMP_MODE_FILTER) {
- ret = -EINVAL;
- goto out;
- }
+ get_seccomp_filter(task);
+ orig = task->seccomp.filter;
+ spin_unlock_irq(&task->sighand->siglock);
- filter = task->seccomp.filter;
- while (filter) {
- filter = filter->prev;
+ count = 0;
+ for (filter = orig; filter; filter = filter->prev)
count++;
- }
if (filter_off >= count) {
ret = -ENOENT;
goto out;
}
- count -= filter_off;
- filter = task->seccomp.filter;
- while (filter && count > 1) {
- filter = filter->prev;
+ count -= filter_off;
+ for (filter = orig; count > 1; filter = filter->prev)
count--;
- }
-
- if (WARN_ON(count != 1 || !filter)) {
- /* The filter tree shouldn't shrink while we're using it. */
- ret = -ENOENT;
- goto out;
- }
fprog = filter->prog->orig_prog;
if (!fprog) {
@@ -912,17 +903,11 @@ long seccomp_get_filter(struct task_struct *task,
unsigned long filter_off,
if (!data)
goto out;
- refcount_inc(&filter->usage);
- spin_unlock_irq(&task->sighand->siglock);
-
if (copy_to_user(data, fprog->filter, bpf_classic_proglen(fprog)))
ret = -EFAULT;
- __put_seccomp_filter(filter);
- return ret;
-
out:
- spin_unlock_irq(&task->sighand->siglock);
+ __put_seccomp_filter(orig);
return ret;
}
#endif
