* [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path
@ 2015-01-29 21:25 Alex Gartrell
2015-01-30 1:02 ` Simon Horman
2015-01-30 8:18 ` Julian Anastasov
0 siblings, 2 replies; 4+ messages in thread
From: Alex Gartrell @ 2015-01-29 21:25 UTC (permalink / raw)
To: wensong, horms, ja; +Cc: lvs-devel, kernel-team, Alex Gartrell
skb->sk can be set in the early_demux path. This can be problematic, as it
may be a time-wait socket, which will blow up in the ip6_output path when
we try to pull out the non-existant pinet6 pointer (the type-punning causes
that pointer to be garbage).
This patch orphans the skb if it's not a local socket, so we no longer have
to worry about running into the time-wait problem from early demux. It
does this by checking for dev, which is unset in that path.
Signed-off-by: Alex Gartrell <agartrell@fb.com>
---
net/netfilter/ipvs/ip_vs_xmit.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 936fd29..9b8c3c4 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -484,6 +484,9 @@ static inline int ip_vs_tunnel_xmit_prepare(struct sk_buff *skb,
if (unlikely(cp->flags & IP_VS_CONN_F_NFCT))
ret = ip_vs_confirm_conntrack(skb);
if (ret == NF_ACCEPT) {
+ /* orphan on LOCAL_IN */
+ if (skb->dev)
+ skb_orphan(skb);
nf_reset(skb);
skb_forward_csum(skb);
}
@@ -502,6 +505,9 @@ static inline int ip_vs_nat_send_or_cont(int pf, struct sk_buff *skb,
else
ip_vs_update_conntrack(skb, cp, 1);
if (!local) {
+ /* orphan on LOCAL_IN */
+ if (skb->dev)
+ skb_orphan(skb);
skb_forward_csum(skb);
NF_HOOK(pf, NF_INET_LOCAL_OUT, skb, NULL, skb_dst(skb)->dev,
dst_output);
@@ -520,6 +526,9 @@ static inline int ip_vs_send_or_cont(int pf, struct sk_buff *skb,
if (likely(!(cp->flags & IP_VS_CONN_F_NFCT)))
ip_vs_notrack(skb);
if (!local) {
+ /* orphan on LOCAL_IN */
+ if (skb->dev)
+ skb_orphan(skb);
skb_forward_csum(skb);
NF_HOOK(pf, NF_INET_LOCAL_OUT, skb, NULL, skb_dst(skb)->dev,
dst_output);
--
Alex Gartrell <agartrell@fb.com>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path
2015-01-29 21:25 [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path Alex Gartrell
@ 2015-01-30 1:02 ` Simon Horman
2015-01-30 1:24 ` Alex Gartrell
2015-01-30 8:18 ` Julian Anastasov
1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2015-01-30 1:02 UTC (permalink / raw)
To: Alex Gartrell; +Cc: wensong, ja, lvs-devel, kernel-team
On Thu, Jan 29, 2015 at 01:25:51PM -0800, Alex Gartrell wrote:
> skb->sk can be set in the early_demux path. This can be problematic, as it
> may be a time-wait socket, which will blow up in the ip6_output path when
> we try to pull out the non-existant pinet6 pointer (the type-punning causes
> that pointer to be garbage).
>
> This patch orphans the skb if it's not a local socket, so we no longer have
> to worry about running into the time-wait problem from early demux. It
> does this by checking for dev, which is unset in that path.
>
> Signed-off-by: Alex Gartrell <agartrell@fb.com>
> ---
Hi Alex,
when you say "blow" what exactly happens? Does the kernel panic?
Assuming this is a bug fix could you provide some information
on when the problem was introduced? The reason is to facilitate
backporting to stable.
Thanks
> net/netfilter/ipvs/ip_vs_xmit.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 936fd29..9b8c3c4 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -484,6 +484,9 @@ static inline int ip_vs_tunnel_xmit_prepare(struct sk_buff *skb,
> if (unlikely(cp->flags & IP_VS_CONN_F_NFCT))
> ret = ip_vs_confirm_conntrack(skb);
> if (ret == NF_ACCEPT) {
> + /* orphan on LOCAL_IN */
> + if (skb->dev)
> + skb_orphan(skb);
> nf_reset(skb);
> skb_forward_csum(skb);
> }
> @@ -502,6 +505,9 @@ static inline int ip_vs_nat_send_or_cont(int pf, struct sk_buff *skb,
> else
> ip_vs_update_conntrack(skb, cp, 1);
> if (!local) {
> + /* orphan on LOCAL_IN */
> + if (skb->dev)
> + skb_orphan(skb);
> skb_forward_csum(skb);
> NF_HOOK(pf, NF_INET_LOCAL_OUT, skb, NULL, skb_dst(skb)->dev,
> dst_output);
> @@ -520,6 +526,9 @@ static inline int ip_vs_send_or_cont(int pf, struct sk_buff *skb,
> if (likely(!(cp->flags & IP_VS_CONN_F_NFCT)))
> ip_vs_notrack(skb);
> if (!local) {
> + /* orphan on LOCAL_IN */
> + if (skb->dev)
> + skb_orphan(skb);
> skb_forward_csum(skb);
> NF_HOOK(pf, NF_INET_LOCAL_OUT, skb, NULL, skb_dst(skb)->dev,
> dst_output);
> --
> Alex Gartrell <agartrell@fb.com>
>
> --
> To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path
2015-01-30 1:02 ` Simon Horman
@ 2015-01-30 1:24 ` Alex Gartrell
0 siblings, 0 replies; 4+ messages in thread
From: Alex Gartrell @ 2015-01-30 1:24 UTC (permalink / raw)
To: Simon Horman; +Cc: wensong, ja, lvs-devel, kernel-team
Hey Simon
On 01/29/2015 05:02 PM, Simon Horman wrote:
> Hi Alex,
>
> when you say "blow" what exactly happens? Does the kernel panic?
> Assuming this is a bug fix could you provide some information
> on when the problem was introduced? The reason is to facilitate
> backporting to stable.
>
The kernel panics upon a garbage value dereference. ip6_skb_dst_mtu
features the following code:
static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
{
struct ipv6_pinfo *np = skb->sk ? inet6_sk(skb->sk) : NULL;
return (np && np->pmtudisc >= IPV6_PMTUDISC_PROBE) ?
skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
}
inet6_sk(skb->sk) expands to
inet_sk(skb->sk)->pinet6 which expands to
((struct inet_sock*)skb->sk)->pinet6
Because skb->sk is not actually an inet_sock, dereferencing at such is
nonsense and gives you a bunch of fields as a pointer, which is totally
bogus. The dereference into the structure to retrieve pmtudisc causes a
kernel panic after that.
We found this with our 3.10 porting effort (from 3.2). This failure was
introduced with early_demux in c7109986d "ipv6: Early TCP socket demux"
(2012-07-26) and has been present since. The underlying problem has
existed forever, but it wasn't possible to trigger it (AFAICT) prior to
that.
In terms of stable kernels, I believe 3.10 needs to be patched but 3.4
does not.
Thanks,
--
Alex Gartrell <agartrell@fb.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path
2015-01-29 21:25 [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path Alex Gartrell
2015-01-30 1:02 ` Simon Horman
@ 2015-01-30 8:18 ` Julian Anastasov
1 sibling, 0 replies; 4+ messages in thread
From: Julian Anastasov @ 2015-01-30 8:18 UTC (permalink / raw)
To: Alex Gartrell; +Cc: wensong, horms, lvs-devel, kernel-team
Hello,
On Thu, 29 Jan 2015, Alex Gartrell wrote:
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -502,6 +505,9 @@ static inline int ip_vs_nat_send_or_cont(int pf, struct sk_buff *skb,
> else
> ip_vs_update_conntrack(skb, cp, 1);
> if (!local) {
> + /* orphan on LOCAL_IN */
> + if (skb->dev)
> + skb_orphan(skb);
If early demux finds one socket in PRE_ROUTING and
later we DNAT the packet to different local real server we should
drop the socket. That is why in previous email I thought
ip_vs_nat_send_or_cont should call skb_orphan() even for
local=true.
Not sure, may be it is possible to get indication
from dnat_handler if packet was changed, so that we can
provide hint to ip_vs_nat_send_or_cont whether to call
skb_orphan only when MASQ mode actually changed daddr/dport,
i.e. when attached socket is not correct anymore.
> skb_forward_csum(skb);
> NF_HOOK(pf, NF_INET_LOCAL_OUT, skb, NULL, skb_dst(skb)->dev,
> dst_output);
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-30 8:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29 21:25 [PATCH net-next,v2] ipvs: orphan skb on LOCAL_IN path Alex Gartrell
2015-01-30 1:02 ` Simon Horman
2015-01-30 1:24 ` Alex Gartrell
2015-01-30 8:18 ` Julian Anastasov
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.