Re: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.wind@foxmail.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
> 
> The current call path of nf_ct_tcp_seqadj_set is the following.
> 
> nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> ->nf_ct_tcp_seqadj_set.
> 
> It couldn't make sure the TCP header is in the linear data part.
> So use the skb_header_pointer instead of the current codes.
> 
> BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> which works in the network layer, it should not assume the transport
> header is in the linear data.
> 
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
>  net/netfilter/nf_conntrack_seqadj.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
> index ef7063e..80394ab 100644
> --- a/net/netfilter/nf_conntrack_seqadj.c
> +++ b/net/netfilter/nf_conntrack_seqadj.c
> @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
>  			  s32 off)
>  {
>  	const struct tcphdr *th;
> +	struct tcphdr tcph;
>  
>  	if (nf_ct_protonum(ct) != IPPROTO_TCP)
>  		return;
>  
> -	th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> +	th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> +	if (!th)
> +		return;

This fix is required since your recent upgrade to check for 4 bytes
instead of 8 bytes from conntrack. Please confirm this.

If so, it would be good to review the NAT code to see if there are
more spots that are not broken since that change...