Message-ID: <CAEY+5yAY12EhntaoDetf7+OTvAHVYKXOJQckZXAowmHGm_p96g@mail.gmail.com>
Date:	Thu, 13 Oct 2011 12:39:33 -0700
From:	Mark Larwill <larwill@...il.com>
To:	netdev@...r.kernel.org
Subject: Help with NULL pointer derefrence in net/ipv4/xfrm4_policy.c

I'm an software developer working for a small firewall company. I am
running into a kernel NULL pointer deference bug (using an older
2.6.35.12 kernel), and noticed a patch submitted that seems like it
might fix my problem. I was hoping someone could help me out a bit. It
is not practical for me to simply upgrade to the latest Linux, rather
I must make a targeted fix.

First a little on my specific issue, then my questions. My problem is
in the file net/ipv4/xfrm4_policy.c in the function
xfrm4_dst_ifdown(), on this line: " if (xdst->u.rt.idev->dev == dev)
{" basically idev is all 0 so xdst->u.rt.idev->dev will crash.

But before I blindly apply the patch I was hoping someone could help
answer some questions on it so I don't make things worse by not
understanding everything. The patch I reference below removes all of
this idev stuff from the kernel. I can see that as long as the if
condition (mentioned above) is met the (xfrm4_dst_ifdown) code
basically goes through a list and assigns the idev structures loopback
devices, so my guess is that for some reason these idev devices are no
longer being used or never were used to begin with. Also similar
behavior is done in the generic ipv4_dst_ifdown() function in
net/ipv4/route.c.

My questions are:
Q1) Why was the idev stuff there to begin with if it was unnecessary?
/ What was it's original purpose?
Q2) Why did we have to assign the devices to the loopback device? (Was
it so no packets would go out during deletion?)
Q3) When, and in what context are the ipv4_dst_ops.ifdown, and
ops->ifdown function pointers called? (What are they doing and why?)
Q4) Between 2.6.35.12 and the patch that removes the idev stuff in
from 11 Nov 2010 in 2.6.38-rc8 are there any other things
removed/changed in between that made it possible to remove the idef
stuff?
Q5) All of these basically lead up to: Why is it safe to remove all of
the idev stuff?
Q6) Is there a safe way I can safely remove this idev stuff or at
least avoid the NULL pointer deference? (Just blindly returning if it
is NULL would be the immediate reaction without thinking about it, but
chances are that's wrong---otherwise why is xfrm4_dst_ifdown written
the way it is?

Any help is greatly appreciated, thanks very much!


--Mark Larwill

REFERENCES:
A)
This post here to kerneltrap.org which seems to describe the old behavior:
http://kerneltrap.org/mailarchive/linux-netdev/2007/9/27/324077

B) The main change I am referring to, and text below
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=72cdd1d971c0deb1619c5c339270570c43647a78


It seems idev field in struct rtable has no special purpose, but adding
extra atomic ops.

We hold refcounts on the device itself (using percpu data, so pretty
cheap in current kernel).

infiniband case is solved using dst.dev instead of idev->dev

Removal of this field means routing without route cache is now using
shared data, percpu data, and only potential contention is a pair of
atomic ops on struct neighbour per forwarded packet.

About 5% speedup on routing test.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists