[PATCH] samples/seccomp: Zero out datastructures based on seccomp_notif_sizes

[PATCH] samples/seccomp: Zero out datastructures based on seccomp_notif_sizes

[Sargun Dhillon]

The sizes by which seccomp_notif and seccomp_notif_resp are allocated are
based on the SECCOMP_GET_NOTIF_SIZES ioctl. This allows for graceful
extension of these datastructures. If userspace zeroes out the
datastructure based on its version, and it is lagging behind the kernel's
version, it will end up sending trailing garbage. On the other hand,
if it is ahead of the kernel version, it will write extra zero space,
and potentially cause corruption.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
Suggested-by: Tycho Andersen <tycho@tycho.ws>
Cc: Kees Cook <keescook@chromium.org>
---
 samples/seccomp/user-trap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/samples/seccomp/user-trap.c b/samples/seccomp/user-trap.c
index 3e31ec0cf4a5..20291ec6489f 100644
--- a/samples/seccomp/user-trap.c
+++ b/samples/seccomp/user-trap.c
@@ -302,10 +302,10 @@ int main(void)
 		resp = malloc(sizes.seccomp_notif_resp);
 		if (!resp)
 			goto out_req;
-		memset(resp, 0, sizeof(*resp));
+		memset(resp, 0, sizes.seccomp_notif_resp);
 
 		while (1) {
-			memset(req, 0, sizeof(*req));
+			memset(req, 0, sizes.seccomp_notif);
 			if (ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, req)) {
 				perror("ioctl recv");
 				goto out_resp;
-- 
2.20.1

Re: [PATCH] samples/seccomp: Zero out datastructures based on seccomp_notif_sizes

[Kees Cook]

On Mon, Dec 30, 2019 at 12:35:03PM -0800, Sargun Dhillon wrote:
> The sizes by which seccomp_notif and seccomp_notif_resp are allocated are
> based on the SECCOMP_GET_NOTIF_SIZES ioctl. This allows for graceful
> extension of these datastructures. If userspace zeroes out the
> datastructure based on its version, and it is lagging behind the kernel's
> version, it will end up sending trailing garbage. On the other hand,
> if it is ahead of the kernel version, it will write extra zero space,
> and potentially cause corruption.
> 
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> Suggested-by: Tycho Andersen <tycho@tycho.ws>
> Cc: Kees Cook <keescook@chromium.org>

Ah, cool. I've rearranged things and split the other patch's samples
changes into this one, etc etc :)

Thanks!

-Kees

> ---
>  samples/seccomp/user-trap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/seccomp/user-trap.c b/samples/seccomp/user-trap.c
> index 3e31ec0cf4a5..20291ec6489f 100644
> --- a/samples/seccomp/user-trap.c
> +++ b/samples/seccomp/user-trap.c
> @@ -302,10 +302,10 @@ int main(void)
>  		resp = malloc(sizes.seccomp_notif_resp);
>  		if (!resp)
>  			goto out_req;
> -		memset(resp, 0, sizeof(*resp));
> +		memset(resp, 0, sizes.seccomp_notif_resp);
>  
>  		while (1) {
> -			memset(req, 0, sizeof(*req));
> +			memset(req, 0, sizes.seccomp_notif);
>  			if (ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, req)) {
>  				perror("ioctl recv");
>  				goto out_resp;
> -- 
> 2.20.1
> 

-- 
Kees Cook