lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 17 Mar 2020 13:38:46 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     sameehj@...zon.com, netdev@...r.kernel.org, bpf@...r.kernel.org,
        zorik@...zon.com, akiyano@...zon.com, gtzalik@...zon.com,
        Toke Høiland-Jørgensen <toke@...e.dk>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        John Fastabend <john.fastabend@...il.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        David Ahern <dsahern@...il.com>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Lorenzo Bianconi <lorenzo@...nel.org>
Subject: Re: [PATCH RFC v1 09/15] xdp: clear grow memory in
 bpf_xdp_adjust_tail()

On Tue, 17 Mar 2020 18:29:53 +0100 Jesper Dangaard Brouer wrote:
> To reviewers: Need some opinions if this is needed?
> 
> (TODO: Squash patch)

I'd vote we don't clear, since we don't clear in adjust head.

We could also add some wrapper around memset() which could be compiled
out based on some CONFIG_ but that could be seen as just moving the
responsibility onto the user..

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 0ceddee0c678..669f29992177 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3432,6 +3432,12 @@ BPF_CALL_2(bpf_xdp_adjust_tail, struct xdp_buff *, xdp, int, offset)
>  	if (unlikely(data_end < xdp->data + ETH_HLEN))
>  		return -EINVAL;
>  
> +	// XXX: To reviewers: How paranoid are we? Do we really need to
> +	/* clear memory area on grow, as in-theory can contain uninit kmem */
> +	if (offset > 0) {
> +		memset(xdp->data_end, 0, offset);
> +	}
> +
>  	xdp->data_end = data_end;
>  
>  	return 0;
> 
> 

Powered by blists - more mailing lists