Re: [PATCH 3/4] seccomp: Add SECCOMP_USER_NOTIF_FLAG_PIDFD to get pidfd on listener trap

On 2020-01-26, Aleksa Sarai [off-list ref] wrote:

On 2020-01-24, Sargun Dhillon [off-list ref] wrote:

quoted

 static long seccomp_notify_recv(struct seccomp_filter *filter,
 				void __user *buf)
 {
 	struct seccomp_knotif *knotif = NULL, *cur;
 	struct seccomp_notif unotif;
+	struct task_struct *group_leader;
+	bool send_pidfd;
 	ssize_t ret;
 
+	if (copy_from_user(&unotif, buf, sizeof(unotif)))
+		return -EFAULT;
 	/* Verify that we're not given garbage to keep struct extensible. */
-	ret = check_zeroed_user(buf, sizeof(unotif));
-	if (ret < 0)
-		return ret;
-	if (!ret)
+	if (unotif.id ||
+	    unotif.pid ||
+	    memchr_inv(&unotif.data, 0, sizeof(unotif.data)) ||
+	    unotif.pidfd)
+		return -EINVAL;

IMHO this check is more confusing than the original check_zeroed_user().
Something like the following is simpler and less prone to forgetting to
add a new field in the future:

	if (memchr_inv(&unotif, 0, sizeof(unotif)))
		return -EINVAL;

Also the check in the patch doesn't ensure that any unnamed padding is
zeroed -- memchr_inv(&unotif, 0, sizeof(unotif)) does.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>