Commit 0e3d8b99 authored by David Johnson's avatar David Johnson

Fix two bugs in skb processing in Linux ipod module.

One was minor (not rolling the ip optional field length into
pskb_may_pull check).  The second was not minor; we weren't
appropriately calling pskb_may_pull to check if the iph + icmph + ipod secret
was in a linear buf... and then we finally ran across a driver for which
the ipod secret did not fully fit in the first skb buffer chunk... so
linearization was actually necessary.

Another way that has been suggested to fix the potential bugs that arise
from linearization, the use of skb_header_pointer, isn't the most
desireable option in this case, since it costs more stack memory *for
each* input ICMP packet (and nearly 100% of the time, it's not an ipod
and we don't care).
parent dc013387
...@@ -176,12 +176,14 @@ static unsigned int ipod_hook_fn( ...@@ -176,12 +176,14 @@ static unsigned int ipod_hook_fn(
struct iphdr *iph; struct iphdr *iph;
struct icmphdr *icmph; struct icmphdr *icmph;
int doit = 0; int doit = 0;
int hlen = 0;
char *data; char *data;
if (!sysctl_ipod_enabled) if (!sysctl_ipod_enabled)
return NF_ACCEPT; return NF_ACCEPT;
if (!pskb_may_pull(skb,sizeof(*iph) + sizeof(*icmph))) hlen = sizeof(*iph);
if (!pskb_may_pull(skb,hlen))
return NF_ACCEPT; return NF_ACCEPT;
iph = (struct iphdr *)skb_network_header(skb); iph = (struct iphdr *)skb_network_header(skb);
...@@ -189,6 +191,14 @@ static unsigned int ipod_hook_fn( ...@@ -189,6 +191,14 @@ static unsigned int ipod_hook_fn(
if (iph->protocol != IPPROTO_ICMP) if (iph->protocol != IPPROTO_ICMP)
return NF_ACCEPT; return NF_ACCEPT;
/* Check again based on the IP header length. */
hlen = iph->ihl * 4 + sizeof(*icmph);
if (!pskb_may_pull(skb,hlen))
return NF_ACCEPT;
/* Grab this again to guard against skb linearization in pskb_may_pull . */
iph = (struct iphdr *)skb_network_header(skb);
/* /*
* icmp_hdr(skb) seems invalid (yet) since the hook is * icmp_hdr(skb) seems invalid (yet) since the hook is
* pre-transport; calculate it manually. * pre-transport; calculate it manually.
...@@ -211,10 +221,15 @@ static unsigned int ipod_hook_fn( ...@@ -211,10 +221,15 @@ static unsigned int ipod_hook_fn(
* enough data or key is otherwise invalid, ignore. * enough data or key is otherwise invalid, ignore.
*/ */
if (IPOD_CHECK_KEY()) { if (IPOD_CHECK_KEY()) {
data = (char *)((char *)icmph + sizeof(*icmph)); if (pskb_may_pull(skb,hlen + sizeof(sysctl_ipod_key) - 1)) {
if (pskb_may_pull(skb,sizeof(sysctl_ipod_key) - 1) /* Guard against linearization, again. */
&& IPOD_VALID_KEY(data)) iph = (struct iphdr *)skb_network_header(skb);
doit = 1; icmph = (struct icmphdr *)((char *)iph + iph->ihl * 4);
data = (char *)((char *)icmph + sizeof(*icmph));
if ((IPOD_VALID_KEY(data)))
doit = 1;
}
} }
else else
doit = 1; doit = 1;
......
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