Applied with some changes, see below.
On Thu, Jul 05, 2018 at 12:01:53PM +0200, Máté Eckl wrote:
> v3: linearize based on layer4 protocol.
> v4: no WARN_ON_ONCE call
Please, next time place these comments...
> -- 8< --
> This patch fixes a silent out-of-bound read possibility that was present
> because of the misuse of this function.
>
> Mostly it was called with a struct udphdr *hp which had only the udphdr
> part linearized by the skb_header_pointer, however
> nf_tproxy_get_sock_v{4,6} uses it as a tcphdr pointer, so some reads for
> tcp specific attributes may be invalid.
>
> Fixes: a583636a83ea ("inet: refactor inet[6]_lookup functions to take skb")
> Signed-off-by: Máté Eckl <ecklm94@xxxxxxxxx>
> ---
*Right here*
'git am' ignores everything coming after ---, so it's very you want to
place volatile comments like the one above.
More comments below.
> include/net/netfilter/nf_tproxy.h | 4 ++--
> net/ipv4/netfilter/nf_tproxy_ipv4.c | 16 +++++++++++-----
> net/ipv6/netfilter/nf_tproxy_ipv6.c | 16 +++++++++++-----
> net/netfilter/xt_TPROXY.c | 8 ++++----
> 4 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tproxy.h b/include/net/netfilter/nf_tproxy.h
> index 9754a50ecde9..4cc64c8446eb 100644
> --- a/include/net/netfilter/nf_tproxy.h
> +++ b/include/net/netfilter/nf_tproxy.h
> @@ -64,7 +64,7 @@ nf_tproxy_handle_time_wait4(struct net *net, struct sk_buff *skb,
> * belonging to established connections going through that one.
> */
> struct sock *
> -nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, void *hp,
> +nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb,
> const u8 protocol,
> const __be32 saddr, const __be32 daddr,
> const __be16 sport, const __be16 dport,
> @@ -103,7 +103,7 @@ nf_tproxy_handle_time_wait6(struct sk_buff *skb, int tproto, int thoff,
> struct sock *sk);
>
> struct sock *
> -nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff, void *hp,
> +nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff,
> const u8 protocol,
> const struct in6_addr *saddr, const struct in6_addr *daddr,
> const __be16 sport, const __be16 dport,
> diff --git a/net/ipv4/netfilter/nf_tproxy_ipv4.c b/net/ipv4/netfilter/nf_tproxy_ipv4.c
> index 805e83ec3ad9..e2559a1cdbf4 100644
> --- a/net/ipv4/netfilter/nf_tproxy_ipv4.c
> +++ b/net/ipv4/netfilter/nf_tproxy_ipv4.c
> @@ -37,7 +37,7 @@ nf_tproxy_handle_time_wait4(struct net *net, struct sk_buff *skb,
> * to a listener socket if there's one */
> struct sock *sk2;
>
> - sk2 = nf_tproxy_get_sock_v4(net, skb, hp, iph->protocol,
> + sk2 = nf_tproxy_get_sock_v4(net, skb, iph->protocol,
> iph->saddr, laddr ? laddr : iph->daddr,
> hp->source, lport ? lport : hp->dest,
> skb->dev, NF_TPROXY_LOOKUP_LISTENER);
> @@ -71,24 +71,27 @@ __be32 nf_tproxy_laddr4(struct sk_buff *skb, __be32 user_laddr, __be32 daddr)
> EXPORT_SYMBOL_GPL(nf_tproxy_laddr4);
>
> struct sock *
> -nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, void *hp,
> +nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb,
> const u8 protocol,
> const __be32 saddr, const __be32 daddr,
> const __be16 sport, const __be16 dport,
> const struct net_device *in,
> const enum nf_tproxy_lookup_t lookup_type)
> {
> + struct tcphdr _hdr, *hp;
> struct sock *sk;
> - struct tcphdr *tcph;
>
> switch (protocol) {
> case IPPROTO_TCP:
I have placed the 'struct tcphdr _hdr, *hp;' here, for clarify so...
> + hp = skb_header_pointer(skb, ip_hdrlen(skb),
> + sizeof(struct tcphdr), &_hdr);
> + if (hp == NULL)
> + return NULL;
> switch (lookup_type) {
> case NF_TPROXY_LOOKUP_LISTENER:
> - tcph = hp;
> sk = inet_lookup_listener(net, &tcp_hashinfo, skb,
> ip_hdrlen(skb) +
> - __tcp_hdrlen(tcph),
> + __tcp_hdrlen(hp),
> saddr, sport,
> daddr, dport,
> in->ifindex, 0);
> @@ -111,6 +114,9 @@ nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, void *hp,
> }
> break;
> case IPPROTO_UDP:
> + /* hp and _hdr is not used here so skb_header_pointer do not
> + * need to be called
> + */
We can skip this comment. Hope you don't mind the mangling.
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
