On Tue, 2 Oct 2012, Linux Kernel Mailing List wrote:
> Gitweb:     
> http://git.kernel.org/linus/;a=commit;h=c12b395a46646bab69089ce7016ac78177f6001f
> Commit:     c12b395a46646bab69089ce7016ac78177f6001f
> Parent:     b7bc2a5b5bd99b216c3e5fe68c7f45c684ab5745
> Author:     [email protected] <[email protected]>
> AuthorDate: Fri Aug 10 00:51:50 2012 +0000
> Committer:  David S. Miller <[email protected]>
> CommitDate: Tue Aug 14 14:28:32 2012 -0700
> 
>     gre: Support GRE over IPv6
>     
>     GRE over IPv6 implementation.
>     
>     Signed-off-by: Dmitry Kozlov <[email protected]>
>     Signed-off-by: David S. Miller <[email protected]>

> --- /dev/null
> +++ b/net/ipv6/ip6_gre.c

> +static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb,
> +                      struct net_device *dev,
> +                      __u8 dsfield,
> +                      struct flowi6 *fl6,
> +                      int encap_limit,
> +                      __u32 *pmtu)
> +{
> +     struct net *net = dev_net(dev);
> +     struct ip6_tnl *tunnel = netdev_priv(dev);
> +     struct net_device *tdev;    /* Device to other host */
> +     struct ipv6hdr  *ipv6h;     /* Our new IP header */
> +     unsigned int max_headroom;  /* The extra header space needed */

max_headroom is not initialized

> +     mtu = dst_mtu(dst) - sizeof(*ipv6h);
> +     if (encap_limit >= 0) {
> +             max_headroom += 8;

Hence gcc (4.1.2) rightfully complains:

net/ipv6/ip6_gre.c: In function ‘ip6gre_xmit2’:
net/ipv6/ip6_gre.c:713: warning: ‘max_headroom’ is used uninitialized in this 
function

However, initializing max_headroom to zero at the top, or replacing the line
above by "max_headroom = 8" doesn't seem to be the right fix...

> +             mtu -= 8;
> +     }
> +     if (mtu < IPV6_MIN_MTU)
> +             mtu = IPV6_MIN_MTU;
> +     if (skb_dst(skb))
> +             skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
> +     if (skb->len > mtu) {
> +             *pmtu = mtu;
> +             err = -EMSGSIZE;
> +             goto tx_err_dst_release;
> +     }
> +
> +     if (tunnel->err_count > 0) {
> +             if (time_before(jiffies,
> +                             tunnel->err_time + IP6TUNNEL_ERR_TIMEO)) {
> +                     tunnel->err_count--;
> +
> +                     dst_link_failure(skb);
> +             } else
> +                     tunnel->err_count = 0;
> +     }
> +
> +     max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen + dst->header_len;

... as max_headroom is overwritten here anyway?

So something is really wrong here. What is the right fix?

This issue is present in current mainline as of v3.7.

Gr{oetje,eeting}s,

                                                Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                                            -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to