Message ID | 20200731044838.213975-1-yepeilin.cs@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [Linux-kernel-mentees,net] openvswitch: Prevent kernel-infoleak in ovs_ct_put_key() | expand |
From: Peilin Ye <yepeilin.cs@gmail.com> Date: Fri, 31 Jul 2020 00:48:38 -0400 > ovs_ct_put_key() is potentially copying uninitialized kernel stack memory > into socket buffers, since the compiler may leave a 3-byte hole at the end > of `struct ovs_key_ct_tuple_ipv4` and `struct ovs_key_ct_tuple_ipv6`. Fix > it by initializing `orig` with memset(). > > Cc: stable@vger.kernel.org Please don't CC: stable for networking fixes. > Fixes: 9dd7f8907c37 ("openvswitch: Add original direction conntrack tuple to sw_flow_key.") > Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> Applied and queued up for -stable, thank you.
On Mon, Aug 03, 2020 at 03:10:38PM -0700, David Miller wrote: > From: Peilin Ye <yepeilin.cs@gmail.com> > Date: Fri, 31 Jul 2020 00:48:38 -0400 > > > ovs_ct_put_key() is potentially copying uninitialized kernel stack memory > > into socket buffers, since the compiler may leave a 3-byte hole at the end > > of `struct ovs_key_ct_tuple_ipv4` and `struct ovs_key_ct_tuple_ipv6`. Fix > > it by initializing `orig` with memset(). > > > > Cc: stable@vger.kernel.org > > Please don't CC: stable for networking fixes. Sorry, I didn't know about that. > > Fixes: 9dd7f8907c37 ("openvswitch: Add original direction conntrack tuple to sw_flow_key.") > > Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > Applied and queued up for -stable, thank you. Thank you for reviewing the patch! Peilin Ye
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 4340f25fe390..98d393e70de3 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -276,10 +276,6 @@ void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key) ovs_ct_update_key(skb, NULL, key, false, false); } -#define IN6_ADDR_INITIALIZER(ADDR) \ - { (ADDR).s6_addr32[0], (ADDR).s6_addr32[1], \ - (ADDR).s6_addr32[2], (ADDR).s6_addr32[3] } - int ovs_ct_put_key(const struct sw_flow_key *swkey, const struct sw_flow_key *output, struct sk_buff *skb) { @@ -301,24 +297,30 @@ int ovs_ct_put_key(const struct sw_flow_key *swkey, if (swkey->ct_orig_proto) { if (swkey->eth.type == htons(ETH_P_IP)) { - struct ovs_key_ct_tuple_ipv4 orig = { - output->ipv4.ct_orig.src, - output->ipv4.ct_orig.dst, - output->ct.orig_tp.src, - output->ct.orig_tp.dst, - output->ct_orig_proto, - }; + struct ovs_key_ct_tuple_ipv4 orig; + + memset(&orig, 0, sizeof(orig)); + orig.ipv4_src = output->ipv4.ct_orig.src; + orig.ipv4_dst = output->ipv4.ct_orig.dst; + orig.src_port = output->ct.orig_tp.src; + orig.dst_port = output->ct.orig_tp.dst; + orig.ipv4_proto = output->ct_orig_proto; + if (nla_put(skb, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4, sizeof(orig), &orig)) return -EMSGSIZE; } else if (swkey->eth.type == htons(ETH_P_IPV6)) { - struct ovs_key_ct_tuple_ipv6 orig = { - IN6_ADDR_INITIALIZER(output->ipv6.ct_orig.src), - IN6_ADDR_INITIALIZER(output->ipv6.ct_orig.dst), - output->ct.orig_tp.src, - output->ct.orig_tp.dst, - output->ct_orig_proto, - }; + struct ovs_key_ct_tuple_ipv6 orig; + + memset(&orig, 0, sizeof(orig)); + memcpy(orig.ipv6_src, output->ipv6.ct_orig.src.s6_addr32, + sizeof(orig.ipv6_src)); + memcpy(orig.ipv6_dst, output->ipv6.ct_orig.dst.s6_addr32, + sizeof(orig.ipv6_dst)); + orig.src_port = output->ct.orig_tp.src; + orig.dst_port = output->ct.orig_tp.dst; + orig.ipv6_proto = output->ct_orig_proto; + if (nla_put(skb, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, sizeof(orig), &orig)) return -EMSGSIZE;
ovs_ct_put_key() is potentially copying uninitialized kernel stack memory into socket buffers, since the compiler may leave a 3-byte hole at the end of `struct ovs_key_ct_tuple_ipv4` and `struct ovs_key_ct_tuple_ipv6`. Fix it by initializing `orig` with memset(). Cc: stable@vger.kernel.org Fixes: 9dd7f8907c37 ("openvswitch: Add original direction conntrack tuple to sw_flow_key.") Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> --- Reference: https://lwn.net/Articles/417989/ $ pahole -C "ovs_key_ct_tuple_ipv4" net/openvswitch/conntrack.o struct ovs_key_ct_tuple_ipv4 { __be32 ipv4_src; /* 0 4 */ __be32 ipv4_dst; /* 4 4 */ __be16 src_port; /* 8 2 */ __be16 dst_port; /* 10 2 */ __u8 ipv4_proto; /* 12 1 */ /* size: 16, cachelines: 1, members: 5 */ /* padding: 3 */ /* last cacheline: 16 bytes */ }; $ pahole -C "ovs_key_ct_tuple_ipv6" net/openvswitch/conntrack.o struct ovs_key_ct_tuple_ipv6 { __be32 ipv6_src[4]; /* 0 16 */ __be32 ipv6_dst[4]; /* 16 16 */ __be16 src_port; /* 32 2 */ __be16 dst_port; /* 34 2 */ __u8 ipv6_proto; /* 36 1 */ /* size: 40, cachelines: 1, members: 5 */ /* padding: 3 */ /* last cacheline: 40 bytes */ }; net/openvswitch/conntrack.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)