Re: [PATCH v8 1/2] seccomp: add a return code to trap to userspace

[Oleg Nesterov <oleg@redhat.com>]

On 10/29, Tycho Andersen wrote:
>
> +static long seccomp_notify_recv(struct seccomp_filter *filter,
> +				void __user *buf)
> +{
> +	struct seccomp_knotif *knotif = NULL, *cur;
> +	struct seccomp_notif unotif;
> +	ssize_t ret;
> +
> +	memset(&unotif, 0, sizeof(unotif));
> +
> +	ret = down_interruptible(&filter->notif->request);
> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_lock(&filter->notify_lock);
> +	list_for_each_entry(cur, &filter->notif->notifications, list) {
> +		if (cur->state == SECCOMP_NOTIFY_INIT) {
> +			knotif = cur;
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * If we didn't find a notification, it could be that the task was
> +	 * interrupted by a fatal signal between the time we were woken and
> +	 * when we were able to acquire the rw lock.
> +	 *
> +	 * This is the place where we handle the extra high semaphore count
> +	 * mentioned in seccomp_do_user_notification().
> +	 */
> +	if (!knotif) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	unotif.id = knotif->id;
> +	unotif.pid = task_pid_vnr(knotif->task);
> +	if (knotif->signaled)
> +		unotif.flags |= SECCOMP_NOTIF_FLAG_SIGNALED;
> +	unotif.data = *(knotif->data);

Tycho, I forgot everything about seccomp, most probably I am wrong but let me
ask anyway.

__seccomp_filter(SECCOMP_RET_TRACE) does

		/*
		 * Recheck the syscall, since it may have changed. This
		 * intentionally uses a NULL struct seccomp_data to force
		 * a reload of all registers. This does not goto skip since
		 * a skip would have already been reported.
		 */
		if (__seccomp_filter(this_syscall, NULL, true))
			return -1;

and the next seccomp_run_filters() can return SECCOMP_RET_USER_NOTIF, right?
seccomp_do_user_notification() doesn't check recheck_after_trace and it simply
does n.data = sd.

Doesn't this mean that "unotif.data = *(knotif->data)" can hit NULL ?

seccomp_run_filters() does populate_seccomp_data() in this case, but this
won't affect "seccomp_data *sd" passed to seccomp_do_user_notification().

Oleg.