From: Li Wei <lw@cn.fujitsu.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH] ipv4: fix a bug in SRR option matching.
Date: Wed, 09 Nov 2011 15:37:55 +0800 [thread overview]
Message-ID: <4EBA2DD3.1070204@cn.fujitsu.com> (raw)
In-Reply-To: <20111108.120621.691425261290061620.davem@davemloft.net>
> From: Li Wei <lw@cn.fujitsu.com>
> Date: Tue, 08 Nov 2011 15:56:40 +0800
>
>> Since commit 7be799a7 (ipv4: Remove rt->rt_dst reference from
>> ip_forward_options()) and commit 0374d9ce (ipv4: Kill spurious
>> write to iph->daddr in ip_forward_options()) we use iph->daddr
>> for SRR option matching and assume iph->daddr equals to rt->rt_dst,
>> Unfortunately skb_rtable(skb) has been updated in ip_options_rcv_srr()
>> for the nexthop in SRR option but iph->daddr *not* updated,
>> We should use the updated rt->rt_dst for SRR option matching
>> and update iph->daddr here.
>>
>> Signed-off-by: Li Wei <lw@cn.fujitsu.com>
>
> Please replace this by whatever logic ip_options_rcv_srr() uses to
> determine the destination address.
>
> I would strongly encourage you, when fixing bugs like this, to use
> as a hint the intentions of the commit which introduced the bug. And
> try as hard as possible to retain the goals of the guilty commit.
>
> In this case, that means not introducing references to rt->rt_dst
> back into the code.
>
> Thank you.
>
>
Thank you for your advice, I reviewed the code again think that as you said
in commit def57687, "No matter what kind of header mangling occurs due to IP
options processing, rt->rt_dst will always equal iph->daddr in the packet",
iph->daddr in ip_options_rcv_srr() should be updated either as skb_rtable(skb)
has been updated for 'nexthop'. So we can elide all rt->rt_dst reference
in ip_forward() and ip_forward_options().
I will submit another patch to fix this bug.
prev parent reply other threads:[~2011-11-09 7:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-08 7:56 [PATCH] ipv4: fix a bug in SRR option matching Li Wei
2011-11-08 17:06 ` David Miller
2011-11-09 7:37 ` Li Wei [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EBA2DD3.1070204@cn.fujitsu.com \
--to=lw@cn.fujitsu.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.