Philip: Similarly, skim down to the econet bug. ]
Dawson Engler writes:
> [BUG] sends sk raw to a bunch of other routines. doesn't seem good.
> /u2/engler/mc/oses/linux/2.4.4-ac8/net/ipv4/ip_output.c:520:ip_build_xmit_slow: ERROR:INTERNAL_NULL:502:520: [type=set] (set at line 502) Dereferencing NULL ptr "sk" illegally!
>
> /*
> * Begin outputting the bytes.
> */
>
> Start --->
> id = (sk ? sk->protinfo.af_inet.id++ : 0);
>
> ... DELETED 12 lines ...
>
>
> /*
> * Fill in the control structures
> */
>
> Error --->
> skb->priority = sk->priority;
> skb->dst = dst_clone(&rt->u.dst);
> skb_reserve(skb, hh_len);
Actually, sk is not allowed to be NULL here, the "(sk ?" bit is bogus
and was cut and pasted from elsewhere. I've removed it in my tree.
And if you look a couple lines previous it is blindly dereferenced,
this should have been a clue :-)
> ---------------------------------------------------------
> [BUG] hard to follow but seems like it.
> /u2/engler/mc/oses/linux/2.4.4-ac8/net/ipv4/fib_frontend.c:272:fib_validate_source: ERROR:INTERNAL_NULL:239:272: [type=set] (set at line 239) Dereferencing NULL ptr "itag" illegally!
> if (fib_lookup(&key, &res))
> goto last_resort;
> if (res.type != RTN_UNICAST)
> goto e_inval_res;
> *spec_dst = FIB_RES_PREFSRC(res);
> Start --->
> if (itag)
>
> ... DELETED 27 lines ...
>
>
> last_resort:
> if (rpf)
> goto e_inval;
> *spec_dst = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
> Error --->
> *itag = 0;
> return 0;
Another case like the previous, no caller sends in a NULL itag.
Check removed in my sources...
> [BUG] contradiction
> /u2/engler/mc/oses/linux/2.4.4/net/ipv4/ip_options.c:257:ip_options_compile: ERROR:INTERNAL_NULL:252:257: [type=set] (set at line 252) Dereferencing NULL ptr "skb" illegally! [val=500]
> int l;
> unsigned char * iph;
> unsigned char * optptr;
> int optlen;
> unsigned char * pp_ptr = NULL;
> Start --->
> struct rtable *rt = skb ? (struct rtable*)skb->dst : NULL;
>
> if (!opt) {
> opt = &(IPCB(skb)->opt);
> memset(opt, 0, sizeof(struct ip_options));
> Error --->
> iph = skb->nh.raw;
> opt->optlen = ((struct iphdr *)iph)->ihl*4 - sizeof(struct iphdr);
> optptr = iph + sizeof(struct iphdr);
> opt->is_data = 0;
How is this a contradiction? The first thing sets 'rt' to NULL or
not, then we test '!opt' not '!rt' which is what I think you are
seeing :-)
If SKB is NULL, opt must be non-NULL and also opt->is_data must be
set in this case. That is the invariant, and even though not %100
explicitly tested, this is what the comment above is trying to say.
At best the missing minor checks should be BUG() checks because they
are illegal states.
> [BUG] contradiction
> /u2/engler/mc/oses/linux/2.4.4/net/core/neighbour.c:317:neigh_create: ERROR:INTERNAL_NULL:312:317: [type=set] (set at line 312) Dereferencing NULL ptr "parms" illegally! [val=500]
> return ERR_PTR(error);
> }
>
> /* Device specific setup. */
> if (n->parms && n->parms->neigh_setup &&
> Start --->
> (error = n->parms->neigh_setup(n)) < 0) {
> neigh_release(n);
> return ERR_PTR(error);
> }
>
> Error --->
> n->confirmed = jiffies - (n->parms->base_reachable_time<<1);
>
> hash_val = tbl->hash(pkey, dev);
I've added the obvious check, but I suspect the more correct fix is
to disallow NULL n->parms (and as a consequence disallow NULL
tbl->constructor since that constructor is the thing that hooks
in the n->parms).
Really, I didn't catch anyone sending a NULL parms in during a
quick perusal, and honestly it would crash instantly on non-x86
systems if it were NULL. (Unfortunately, at least during boot,
the NULL address is actually mapped in the kernel on x86 :-( ).
> ---------------------------------------------------------
> [BUG] contradicts
> /u2/engler/mc/oses/linux/2.4.4/net/ipv4/ip_input.c:325:ip_rcv_finish: ERROR:INTERNAL_NULL:319:325: [type=set] (set at line 319) Dereferencing NULL ptr "dst" illegally! [val=600]
>
> /*
> * Initialise the virtual path cache for the packet. It describes
> * how the packet travels inside Linux networking.
> */
> Start --->
> if (skb->dst == NULL) {
> if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
> goto drop;
> }
>
> #ifdef CONFIG_NET_CLS_ROUTE
> Error --->
> if (skb->dst->tclassid) {
> struct ip_rt_acct *st = ip_rt_acct + 256*smp_processor_id();
> u32 idx = skb->dst->tclassid;
> st[idx&0xFF].o_packets++;
This one is OK.
The side effect of a successful ip_route_input call is that skb->dst
gets set to a non-NULL value.
> ---------------------------------------------------------
> [BUG] contradicts.
> /u2/engler/mc/oses/linux/2.4.4/net/ipv4/devinet.c:544:devinet_ioctl: ERROR:INTERNAL_NULL:537:544: [type=set] (set at line 537) Dereferencing NULL ptr "ifa" illegally! [val=700]
> for (ifap=&in_dev->ifa_list; (ifa=*ifap) != NULL; ifap=&ifa->ifa_next)
> if (strcmp(ifr.ifr_name, ifa->ifa_label) == 0)
> break;
> }
>
> Start --->
> if (ifa == NULL && cmd != SIOCSIFADDR && cmd != SIOCSIFFLAGS) {
> ret = -EADDRNOTAVAIL;
> goto done;
> }
>
> switch(cmd) {
> case SIOCGIFADDR: /* Get interface address */
> Error --->
> sin->sin_addr.s_addr = ifa->ifa_local;
> goto rarok;
>
> case SIOCGIFBRDADDR: /* Get the broadcast address */
> ---------------------------------------------------------
> [BUG] should be ||?
> /u2/engler/mc/oses/linux/2.4.4/net/decnet/dn_dev.c:500:dn_dev_ioctl: ERROR:INTERNAL_NULL:493:500: [type=set] (set at line 493) Dereferencing NULL ptr "ifa" illegally! [val=700]
> for (ifap = &dn_db->ifa_list; (ifa=*ifap) != NULL; ifap = &ifa->ifa_next)
> if (strcmp(ifr->ifr_name, ifa->ifa_label) == 0)
> break;
> }
>
> Start --->
> if (ifa == NULL && cmd != SIOCSIFADDR) {
> ret = -EADDRNOTAVAIL;
> goto done;
> }
>
> switch(cmd) {
> case SIOCGIFADDR:
> Error --->
> *((dn_address *)sdn->sdn_nodeaddr) = ifa->ifa_local;
> goto rarok;
>
> case SIOCSIFADDR:
Checker is wrong in these two cases.
The "error" code you point to cannot be reached if cmd == SIOCGIFADDR
(for the dn_dev.c, likewise SIOCSIFADDR+SIOCSIFFLAGS in the devinet.c
case). In the dn_dev.c case, SIOCSIFADDR in that switch statement may
run if ifa == NULL.
Something is screwed with checker's flow graphs perhaps? Else it
isn't doing exhaustive enough reachability determination.
> ---------------------------------------------------------
> [BUG] contradicts
> /u2/engler/mc/oses/linux/2.4.4/net/decnet/dn_route.c:808:dn_route_output_slow: ERROR:INTERNAL_NULL:798:808: [type=set] (set at line 798) Dereferencing NULL ptr "neigh" illegally! [val=1000]
>
> rt->key.saddr = src;
> rt->rt_saddr = src;
> rt->key.daddr = dst;
> rt->rt_daddr = dst;
> Start --->
> rt->key.oif = neigh ? neigh->dev->ifindex : -1;
> rt->key.iif = 0;
> rt->key.fwmark = 0;
>
> rt->u.dst.neighbour = neigh;
> rt->u.dst.dev = neigh ? neigh->dev : NULL;
> rt->u.dst.lastuse = jiffies;
> rt->u.dst.output = dn_output;
> rt->u.dst.input = dn_rt_bug;
>
> Error --->
> if (dn_dev_islocal(neigh->dev, rt->rt_daddr))
> rt->u.dst.input = dn_nsp_rx;
>
> hash = dn_hash(rt->key.saddr, rt->key.daddr);
Yeah, this one is wrong, I'll leave this to Steve Whitehouse though,
the DecNET maintainer.
> [BUG] seems like it, unless there is a weird DP going on.
> /u2/engler/mc/oses/linux/2.4.4/net/econet/af_econet.c:310:econet_sendmsg: ERROR:INTERNAL_NULL:268:310: [type=set] (set at line 268) Dereferencing NULL ptr "saddr" illegally! [val=4200]
>
> /*
> * Get and verify the address.
> */
>
> Start --->
> if (saddr == NULL) {
>
> ... DELETED 36 lines ...
>
> skb_reserve(skb, (dev->hard_header_len+15)&~15);
> skb->nh.raw = skb->data;
>
> eb = (struct ec_cb *)&skb->cb;
>
> Error --->
> eb->cookie = saddr->cookie;
> eb->sec = *saddr;
> eb->sent = ec_tx_done;
Yep, this one is correct. I don't know enough about econet to fix it
though... Philip?
Later,
David S. Miller
davem@redhat.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/