netdev - Re: [PATCH RFC v1 09/15] xdp: clear grow memory in bpf_xdp_adjust

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 linux-cve-announce 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]

Message-ID: <87v9n2koqt.fsf@toke.dk>
Date:   Wed, 18 Mar 2020 10:15:38 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>, sameehj@...zon.com
Cc:     Jesper Dangaard Brouer <brouer@...hat.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, zorik@...zon.com, akiyano@...zon.com,
        gtzalik@...zon.com, 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()

Jesper Dangaard Brouer <brouer@...hat.com> writes:

> To reviewers: Need some opinions if this is needed?
>
> (TODO: Squash patch)
> ---
>  net/core/filter.c |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> 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);
> +	}

This memory will usually be recycled through page_pool or equivalent,
right? So couldn't we clear the pages when they are first allocated?
That way, the only data that would be left there would be packet data
from previous packets...

-Toke