Commit 41bdb8a0 authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'amt-fix-several-bugs-in-amt_rcv'

Taehee Yoo says:

====================
amt: fix several bugs in amt_rcv()

This series fixes bugs in amt_rcv().

First patch fixes pskb_may_pull() issue.
Some functions missed to call pskb_may_pull() and uses wrong
parameter of pskb_may_pull().

Second patch fixes possible null-ptr-deref in amt_rcv().
If there is no amt private data in sock, skb will be freed.
And it increases stats.
But in order to increase stats, amt private data is needed.
So, uninitialised pointer will be used at that point.

Third patch fixes wrong definition of type_str[] in amt.c
====================

Link: https://lore.kernel.org/r/20220602140108.18329-1-ap420073@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents c76acfb7 d7970039
...@@ -51,6 +51,7 @@ static char *status_str[] = { ...@@ -51,6 +51,7 @@ static char *status_str[] = {
}; };
static char *type_str[] = { static char *type_str[] = {
"", /* Type 0 is not defined */
"AMT_MSG_DISCOVERY", "AMT_MSG_DISCOVERY",
"AMT_MSG_ADVERTISEMENT", "AMT_MSG_ADVERTISEMENT",
"AMT_MSG_REQUEST", "AMT_MSG_REQUEST",
...@@ -2220,8 +2221,7 @@ static bool amt_advertisement_handler(struct amt_dev *amt, struct sk_buff *skb) ...@@ -2220,8 +2221,7 @@ static bool amt_advertisement_handler(struct amt_dev *amt, struct sk_buff *skb)
struct amt_header_advertisement *amta; struct amt_header_advertisement *amta;
int hdr_size; int hdr_size;
hdr_size = sizeof(*amta) - sizeof(struct amt_header); hdr_size = sizeof(*amta) + sizeof(struct udphdr);
if (!pskb_may_pull(skb, hdr_size)) if (!pskb_may_pull(skb, hdr_size))
return true; return true;
...@@ -2251,19 +2251,27 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) ...@@ -2251,19 +2251,27 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
struct ethhdr *eth; struct ethhdr *eth;
struct iphdr *iph; struct iphdr *iph;
hdr_size = sizeof(*amtmd) + sizeof(struct udphdr);
if (!pskb_may_pull(skb, hdr_size))
return true;
amtmd = (struct amt_header_mcast_data *)(udp_hdr(skb) + 1); amtmd = (struct amt_header_mcast_data *)(udp_hdr(skb) + 1);
if (amtmd->reserved || amtmd->version) if (amtmd->reserved || amtmd->version)
return true; return true;
hdr_size = sizeof(*amtmd) + sizeof(struct udphdr);
if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_IP), false)) if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_IP), false))
return true; return true;
skb_reset_network_header(skb); skb_reset_network_header(skb);
skb_push(skb, sizeof(*eth)); skb_push(skb, sizeof(*eth));
skb_reset_mac_header(skb); skb_reset_mac_header(skb);
skb_pull(skb, sizeof(*eth)); skb_pull(skb, sizeof(*eth));
eth = eth_hdr(skb); eth = eth_hdr(skb);
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
iph = ip_hdr(skb); iph = ip_hdr(skb);
if (iph->version == 4) { if (iph->version == 4) {
if (!ipv4_is_multicast(iph->daddr)) if (!ipv4_is_multicast(iph->daddr))
return true; return true;
...@@ -2274,6 +2282,9 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) ...@@ -2274,6 +2282,9 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
} else if (iph->version == 6) { } else if (iph->version == 6) {
struct ipv6hdr *ip6h; struct ipv6hdr *ip6h;
if (!pskb_may_pull(skb, sizeof(*ip6h)))
return true;
ip6h = ipv6_hdr(skb); ip6h = ipv6_hdr(skb);
if (!ipv6_addr_is_multicast(&ip6h->daddr)) if (!ipv6_addr_is_multicast(&ip6h->daddr))
return true; return true;
...@@ -2306,8 +2317,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt, ...@@ -2306,8 +2317,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
struct iphdr *iph; struct iphdr *iph;
int hdr_size, len; int hdr_size, len;
hdr_size = sizeof(*amtmq) - sizeof(struct amt_header); hdr_size = sizeof(*amtmq) + sizeof(struct udphdr);
if (!pskb_may_pull(skb, hdr_size)) if (!pskb_may_pull(skb, hdr_size))
return true; return true;
...@@ -2315,22 +2325,27 @@ static bool amt_membership_query_handler(struct amt_dev *amt, ...@@ -2315,22 +2325,27 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
if (amtmq->reserved || amtmq->version) if (amtmq->reserved || amtmq->version)
return true; return true;
hdr_size = sizeof(*amtmq) + sizeof(struct udphdr) - sizeof(*eth); hdr_size -= sizeof(*eth);
if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_TEB), false)) if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_TEB), false))
return true; return true;
oeth = eth_hdr(skb); oeth = eth_hdr(skb);
skb_reset_mac_header(skb); skb_reset_mac_header(skb);
skb_pull(skb, sizeof(*eth)); skb_pull(skb, sizeof(*eth));
skb_reset_network_header(skb); skb_reset_network_header(skb);
eth = eth_hdr(skb); eth = eth_hdr(skb);
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
iph = ip_hdr(skb); iph = ip_hdr(skb);
if (iph->version == 4) { if (iph->version == 4) {
if (!ipv4_is_multicast(iph->daddr))
return true;
if (!pskb_may_pull(skb, sizeof(*iph) + AMT_IPHDR_OPTS + if (!pskb_may_pull(skb, sizeof(*iph) + AMT_IPHDR_OPTS +
sizeof(*ihv3))) sizeof(*ihv3)))
return true; return true;
if (!ipv4_is_multicast(iph->daddr))
return true;
ihv3 = skb_pull(skb, sizeof(*iph) + AMT_IPHDR_OPTS); ihv3 = skb_pull(skb, sizeof(*iph) + AMT_IPHDR_OPTS);
skb_reset_transport_header(skb); skb_reset_transport_header(skb);
skb_push(skb, sizeof(*iph) + AMT_IPHDR_OPTS); skb_push(skb, sizeof(*iph) + AMT_IPHDR_OPTS);
...@@ -2345,15 +2360,17 @@ static bool amt_membership_query_handler(struct amt_dev *amt, ...@@ -2345,15 +2360,17 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
ip_eth_mc_map(iph->daddr, eth->h_dest); ip_eth_mc_map(iph->daddr, eth->h_dest);
#if IS_ENABLED(CONFIG_IPV6) #if IS_ENABLED(CONFIG_IPV6)
} else if (iph->version == 6) { } else if (iph->version == 6) {
struct ipv6hdr *ip6h = ipv6_hdr(skb);
struct mld2_query *mld2q; struct mld2_query *mld2q;
struct ipv6hdr *ip6h;
if (!ipv6_addr_is_multicast(&ip6h->daddr))
return true;
if (!pskb_may_pull(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS + if (!pskb_may_pull(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS +
sizeof(*mld2q))) sizeof(*mld2q)))
return true; return true;
ip6h = ipv6_hdr(skb);
if (!ipv6_addr_is_multicast(&ip6h->daddr))
return true;
mld2q = skb_pull(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS); mld2q = skb_pull(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS);
skb_reset_transport_header(skb); skb_reset_transport_header(skb);
skb_push(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS); skb_push(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS);
...@@ -2389,23 +2406,23 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) ...@@ -2389,23 +2406,23 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
{ {
struct amt_header_membership_update *amtmu; struct amt_header_membership_update *amtmu;
struct amt_tunnel_list *tunnel; struct amt_tunnel_list *tunnel;
struct udphdr *udph;
struct ethhdr *eth; struct ethhdr *eth;
struct iphdr *iph; struct iphdr *iph;
int len; int len, hdr_size;
iph = ip_hdr(skb); iph = ip_hdr(skb);
udph = udp_hdr(skb);
if (__iptunnel_pull_header(skb, sizeof(*udph), skb->protocol, hdr_size = sizeof(*amtmu) + sizeof(struct udphdr);
false, false)) if (!pskb_may_pull(skb, hdr_size))
return true; return true;
amtmu = (struct amt_header_membership_update *)skb->data; amtmu = (struct amt_header_membership_update *)(udp_hdr(skb) + 1);
if (amtmu->reserved || amtmu->version) if (amtmu->reserved || amtmu->version)
return true; return true;
skb_pull(skb, sizeof(*amtmu)); if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false))
return true;
skb_reset_network_header(skb); skb_reset_network_header(skb);
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) { list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) {
...@@ -2426,6 +2443,9 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) ...@@ -2426,6 +2443,9 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
return true; return true;
report: report:
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
iph = ip_hdr(skb); iph = ip_hdr(skb);
if (iph->version == 4) { if (iph->version == 4) {
if (ip_mc_check_igmp(skb)) { if (ip_mc_check_igmp(skb)) {
...@@ -2679,7 +2699,8 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb) ...@@ -2679,7 +2699,8 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
amt = rcu_dereference_sk_user_data(sk); amt = rcu_dereference_sk_user_data(sk);
if (!amt) { if (!amt) {
err = true; err = true;
goto drop; kfree_skb(skb);
goto out;
} }
skb->dev = amt->dev; skb->dev = amt->dev;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment