Bug 8107 - dev->header_cache_update has a random value
Summary: dev->header_cache_update has a random value
Status: CLOSED CODE_FIX
Alias: None
Product: Networking
Classification: Unclassified
Component: Other (show other bugs)
Hardware: i386 Linux
: P2 high
Assignee: Arnaldo Carvalho de Melo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-03-01 11:33 UTC by Chongfeng Hu
Modified: 2008-09-23 08:38 UTC (History)
0 users

See Also:
Kernel Version: 2.6.20
Tree: Mainline
Regression: ---


Attachments

Description Chongfeng Hu 2007-03-01 11:33:02 UTC
Distribution: Kernel 2.6.20

Problem Description:

In struct net_device, there are two fields: hard_header_cache and 
header_cache_update, both of which are function pointers. The third field, 
hard_header, is also a function pointer. Whenever hard_header points to a valid 
function, both hard_header_cache and header_cache_update should have a known 
value, either NULL or a valid function pointer. However, in 
drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
function, and dev->hard_header_cache is assigned a known value (NULL), dev-
>header_cache_update is not set to a known value:

dev->hard_start_xmit = hdlc->xmit;
        dev->hard_header = cisco_hard_header;
        dev->hard_header_cache = NULL;
        dev->type = ARPHRD_CISCO;
        dev->flags = IFF_POINTOPOINT | IFF_NOARP;
        dev->addr_len = 0;

This may cause serious problems when dev->header_cache_update is invoked, 
because it has an uninitialized value.

Steps to reproduce:

I found this suspicious spot with the help of a code-checking tool.
Comment 1 Anonymous Emailer 2007-03-01 14:34:23 UTC
Reply-To: akpm@linux-foundation.org

On Thu, 1 Mar 2007 11:33:05 -0800
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=8107
> 
>            Summary: dev->header_cache_update has a random value
>     Kernel Version: 2.6.20
>             Status: NEW
>           Severity: high
>              Owner: jgarzik@pobox.com
>          Submitter: loveminix@yahoo.com.cn
> 
> 
> Distribution: Kernel 2.6.20
> 
> Problem Description:
> 
> In struct net_device, there are two fields: hard_header_cache and 
> header_cache_update, both of which are function pointers. The third field, 
> hard_header, is also a function pointer. Whenever hard_header points to a valid 
> function, both hard_header_cache and header_cache_update should have a known 
> value, either NULL or a valid function pointer. However, in 
> drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
> net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
> function, and dev->hard_header_cache is assigned a known value (NULL), dev-
> >header_cache_update is not set to a known value:
> 
> dev->hard_start_xmit = hdlc->xmit;
>         dev->hard_header = cisco_hard_header;
>         dev->hard_header_cache = NULL;
>         dev->type = ARPHRD_CISCO;
>         dev->flags = IFF_POINTOPOINT | IFF_NOARP;
>         dev->addr_len = 0;
> 
> This may cause serious problems when dev->header_cache_update is invoked, 
> because it has an uninitialized value.
> 
> Steps to reproduce:
> 
> I found this suspicious spot with the help of a code-checking tool.
> 

Like this?



From: Andrew Morton <akpm@linux-foundation.org>

Fix http://bugzilla.kernel.org/show_bug.cgi?id=8107: we weren't initialising
the header_cache_update field.

Cc: Krzysztof Halasa <khc@pm.waw.pl>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/wan/hdlc_cisco.c |    1 +
 1 files changed, 1 insertion(+)

diff -puN drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update drivers/net/wan/hdlc_cisco.c
--- a/drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update
+++ a/drivers/net/wan/hdlc_cisco.c
@@ -366,6 +366,7 @@ static int cisco_ioctl(struct net_device
 		dev->hard_start_xmit = hdlc->xmit;
 		dev->hard_header = cisco_hard_header;
 		dev->hard_header_cache = NULL;
+		dev->header_cache_update = NULL;
 		dev->type = ARPHRD_CISCO;
 		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
 		dev->addr_len = 0;
_

Comment 2 Andrew Morton 2007-03-01 14:34:50 UTC
I queued a patch.
Comment 3 Anonymous Emailer 2007-03-01 14:37:29 UTC
Reply-To: shemminger@linux-foundation.org

On Thu, 1 Mar 2007 14:34:17 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 1 Mar 2007 11:33:05 -0800
> bugme-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=8107
> > 
> >            Summary: dev->header_cache_update has a random value
> >     Kernel Version: 2.6.20
> >             Status: NEW
> >           Severity: high
> >              Owner: jgarzik@pobox.com
> >          Submitter: loveminix@yahoo.com.cn
> > 
> > 
> > Distribution: Kernel 2.6.20
> > 
> > Problem Description:
> > 
> > In struct net_device, there are two fields: hard_header_cache and 
> > header_cache_update, both of which are function pointers. The third field, 
> > hard_header, is also a function pointer. Whenever hard_header points to a valid 
> > function, both hard_header_cache and header_cache_update should have a known 
> > value, either NULL or a valid function pointer. However, in 
> > drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
> > net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
> > function, and dev->hard_header_cache is assigned a known value (NULL), dev-
> > >header_cache_update is not set to a known value:
> > 
> > dev->hard_start_xmit = hdlc->xmit;
> >         dev->hard_header = cisco_hard_header;
> >         dev->hard_header_cache = NULL;
> >         dev->type = ARPHRD_CISCO;
> >         dev->flags = IFF_POINTOPOINT | IFF_NOARP;
> >         dev->addr_len = 0;
> > 
> > This may cause serious problems when dev->header_cache_update is invoked, 
> > because it has an uninitialized value.
> > 
> > Steps to reproduce:
> > 
> > I found this suspicious spot with the help of a code-checking tool.
> > 
> 
> Like this?

Not necessary, since any network device must already allocated by
alloc_netdev() and it initializes the whole struct to 0 (NULL).

Comment 4 Anonymous Emailer 2007-03-01 14:54:26 UTC
Reply-To: akpm@linux-foundation.org

On Thu, 1 Mar 2007 14:37:27 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> On Thu, 1 Mar 2007 14:34:17 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 1 Mar 2007 11:33:05 -0800
> > bugme-daemon@bugzilla.kernel.org wrote:
> > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=8107
> > > 
> > >            Summary: dev->header_cache_update has a random value
> > >     Kernel Version: 2.6.20
> > >             Status: NEW
> > >           Severity: high
> > >              Owner: jgarzik@pobox.com
> > >          Submitter: loveminix@yahoo.com.cn
> > > 
> > > 
> > > Distribution: Kernel 2.6.20
> > > 
> > > Problem Description:
> > > 
> > > In struct net_device, there are two fields: hard_header_cache and 
> > > header_cache_update, both of which are function pointers. The third field, 
> > > hard_header, is also a function pointer. Whenever hard_header points to a valid 
> > > function, both hard_header_cache and header_cache_update should have a known 
> > > value, either NULL or a valid function pointer. However, in 
> > > drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
> > > net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
> > > function, and dev->hard_header_cache is assigned a known value (NULL), dev-
> > > >header_cache_update is not set to a known value:
> > > 
> > > dev->hard_start_xmit = hdlc->xmit;
> > >         dev->hard_header = cisco_hard_header;
> > >         dev->hard_header_cache = NULL;
> > >         dev->type = ARPHRD_CISCO;
> > >         dev->flags = IFF_POINTOPOINT | IFF_NOARP;
> > >         dev->addr_len = 0;
> > > 
> > > This may cause serious problems when dev->header_cache_update is invoked, 
> > > because it has an uninitialized value.
> > > 
> > > Steps to reproduce:
> > > 
> > > I found this suspicious spot with the help of a code-checking tool.
> > > 
> > 
> > Like this?
> 
> Not necessary, since any network device must already allocated by
> alloc_netdev() and it initializes the whole struct to 0 (NULL).

But ioctl(IF_PROTO_CISCO) can be run multiple times across the lifetime of
a net_device?

Comment 5 Anonymous Emailer 2007-03-01 15:30:29 UTC
Reply-To: shemminger@linux-foundation.org

On Thu, 1 Mar 2007 14:54:23 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 1 Mar 2007 14:37:27 -0800
> Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> 
> > On Thu, 1 Mar 2007 14:34:17 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Thu, 1 Mar 2007 11:33:05 -0800
> > > bugme-daemon@bugzilla.kernel.org wrote:
> > > 
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=8107
> > > > 
> > > >            Summary: dev->header_cache_update has a random value
> > > >     Kernel Version: 2.6.20
> > > >             Status: NEW
> > > >           Severity: high
> > > >              Owner: jgarzik@pobox.com
> > > >          Submitter: loveminix@yahoo.com.cn
> > > > 
> > > > 
> > > > Distribution: Kernel 2.6.20
> > > > 
> > > > Problem Description:
> > > > 
> > > > In struct net_device, there are two fields: hard_header_cache and 
> > > > header_cache_update, both of which are function pointers. The third field, 
> > > > hard_header, is also a function pointer. Whenever hard_header points to a valid 
> > > > function, both hard_header_cache and header_cache_update should have a known 
> > > > value, either NULL or a valid function pointer. However, in 
> > > > drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
> > > > net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
> > > > function, and dev->hard_header_cache is assigned a known value (NULL), dev-
> > > > >header_cache_update is not set to a known value:
> > > > 
> > > > dev->hard_start_xmit = hdlc->xmit;
> > > >         dev->hard_header = cisco_hard_header;
> > > >         dev->hard_header_cache = NULL;
> > > >         dev->type = ARPHRD_CISCO;
> > > >         dev->flags = IFF_POINTOPOINT | IFF_NOARP;
> > > >         dev->addr_len = 0;
> > > > 
> > > > This may cause serious problems when dev->header_cache_update is invoked, 
> > > > because it has an uninitialized value.
> > > > 
> > > > Steps to reproduce:
> > > > 
> > > > I found this suspicious spot with the help of a code-checking tool.
> > > > 
> > > 
> > > Like this?
> > 
> > Not necessary, since any network device must already allocated by
> > alloc_netdev() and it initializes the whole struct to 0 (NULL).
> 
> But ioctl(IF_PROTO_CISCO) can be run multiple times across the lifetime of
> a net_device?

Your right, but so far there is no ioctl to take it out of this mode.
So it is a one way door.

This device never calls hdlc->detach??

Comment 6 Anonymous Emailer 2007-03-01 17:33:15 UTC
Reply-To: davem@davemloft.net

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 1 Mar 2007 14:37:27 -0800

> Not necessary, since any network device must already allocated by
> alloc_netdev() and it initializes the whole struct to 0 (NULL).

It is in this case, unfortunately, HDLC protocols can be registers
several times before the device is brought up and protocol changes
become disallowed.

So you can attach one, then a second one, and the second one has
to explicitly initialize the pointers potentially set by the
first one.

On the other hand, you could say that it's the protocol
->detach() method's responsibility to NULL out these things
instead of leaving references to function pointers of
a module that's about the be unloaded.

Comment 7 Anonymous Emailer 2007-03-01 17:38:48 UTC
Reply-To: davem@davemloft.net

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 1 Mar 2007 15:30:25 -0800

> Your right, but so far there is no ioctl to take it out of this mode.
> So it is a one way door.

You can keep attaching a different new protocol to an HDLC
device until it is brought up.

> This device never calls hdlc->detach??

The HDLC layer calls the protocol ->detach, but the Cisco
HDLC code does not have a detach method.

I'm starting to feel that this is the bug, because as it
stands the Cisco HDLC code leaves stale pointers around
and then if you unload the Cisco HDLC module things can
explode.

Comment 8 Anonymous Emailer 2007-03-02 07:29:14 UTC
Reply-To: khc@pm.waw.pl

Andrew Morton <akpm@linux-foundation.org> writes:

>> However, in 
>> drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
>> net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
>> function, and dev->hard_header_cache is assigned a known value (NULL), dev-
>> >header_cache_update is not set to a known value:

Right, it seems I was never aware of dev->header_cache_update existence.
I wonder where does the non-NULL value come from? Nevermind.

> diff -puN drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update drivers/net/wan/hdlc_cisco.c
> --- a/drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update
> +++ a/drivers/net/wan/hdlc_cisco.c
> @@ -366,6 +366,7 @@ static int cisco_ioctl(struct net_device
>  		dev->hard_start_xmit = hdlc->xmit;
>  		dev->hard_header = cisco_hard_header;
>  		dev->hard_header_cache = NULL;
> +		dev->header_cache_update = NULL;
>  		dev->type = ARPHRD_CISCO;
>  		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
>  		dev->addr_len = 0;
> _

ACK, I think it's the best place.

Is it OK to leave this (and hard_header_cache) set to random value
if dev->hard_header = NULL (as with other protocols)?
Comment 9 Anonymous Emailer 2007-03-02 11:23:30 UTC
Reply-To: davem@davemloft.net

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Fri, 02 Mar 2007 16:29:06 +0100

> Andrew Morton <akpm@linux-foundation.org> writes:
> 
> >> However, in 
> >> drivers/net/wan/hdlc_cisco.c, in function static int cisco_ioctl(struct 
> >> net_device *dev, struct ifreq *ifr), where dev->hard_header is assigned a valid 
> >> function, and dev->hard_header_cache is assigned a known value (NULL), dev-
> >> >header_cache_update is not set to a known value:
> 
> Right, it seems I was never aware of dev->header_cache_update existence.
> I wonder where does the non-NULL value come from? Nevermind.
> 
> > diff -puN drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update drivers/net/wan/hdlc_cisco.c
> > --- a/drivers/net/wan/hdlc_cisco.c~cisco_ioctl-initialise-header_cache_update
> > +++ a/drivers/net/wan/hdlc_cisco.c
> > @@ -366,6 +366,7 @@ static int cisco_ioctl(struct net_device
> >  		dev->hard_start_xmit = hdlc->xmit;
> >  		dev->hard_header = cisco_hard_header;
> >  		dev->hard_header_cache = NULL;
> > +		dev->header_cache_update = NULL;
> >  		dev->type = ARPHRD_CISCO;
> >  		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
> >  		dev->addr_len = 0;
> > _
> 
> ACK, I think it's the best place.

I disagree, you can't leave dangling references to functions
which are potentially inside of unloaded modules, as this code
does.

Rather, HDLC Cisco should implement a proper protocol destructor
method to clean up these function pointers.

Comment 10 Anonymous Emailer 2007-03-02 15:38:11 UTC
Reply-To: khc@pm.waw.pl

Switching HDLC devices from Ethernet-framing mode caused stale ethernet
function assignments within net_device.

Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>

diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index db354e0..f6e6b63 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/net/wan/hdlc.c
@@ -38,7 +38,7 @@
 #include <linux/hdlc.h>
 
 
-static const char* version = "HDLC support module revision 1.20";
+static const char* version = "HDLC support module revision 1.21";
 
 #undef DEBUG_LINK
 
@@ -222,19 +222,30 @@ int hdlc_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	return -EINVAL;
 }
 
+static void hdlc_setup_dev(struct net_device *dev)
+{
+/* Re-init all variables changed by HDLC protocol drivers,
+   including ether_setup() called from hdlc_raw_eth.c. */
+	dev->get_stats		 = hdlc_get_stats;
+	dev->flags		 = IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		 = HDLC_MAX_MTU;
+	dev->type		 = ARPHRD_RAWHDLC;
+	dev->hard_header_len	 = 16;
+	dev->addr_len		 = 0;
+	dev->hard_header	 = NULL;
+	dev->rebuild_header	 = NULL;
+	dev->set_mac_address	 = NULL;
+	dev->hard_header_cache	 = NULL;
+	dev->header_cache_update = NULL;
+	dev->change_mtu		 = hdlc_change_mtu;
+	dev->hard_header_parse	 = NULL;
+}
+
 void hdlc_setup(struct net_device *dev)
 {
 	hdlc_device *hdlc = dev_to_hdlc(dev);
 
-	dev->get_stats = hdlc_get_stats;
-	dev->change_mtu = hdlc_change_mtu;
-	dev->mtu = HDLC_MAX_MTU;
-
-	dev->type = ARPHRD_RAWHDLC;
-	dev->hard_header_len = 16;
-
-	dev->flags = IFF_POINTOPOINT | IFF_NOARP;
-
+	hdlc_setup_dev(dev);
 	hdlc->carrier = 1;
 	hdlc->open = 0;
 	spin_lock_init(&hdlc->state_lock);
@@ -294,6 +305,7 @@ void detach_hdlc_protocol(struct net_device *dev)
 	}
 	kfree(hdlc->state);
 	hdlc->state = NULL;
+	hdlc_setup_dev(dev);
 }
 
 
diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index b0bc5dd..c9664fd 100644
--- a/drivers/net/wan/hdlc_cisco.c
+++ b/drivers/net/wan/hdlc_cisco.c
@@ -365,10 +365,7 @@ static int cisco_ioctl(struct net_device *dev, struct ifreq *ifr)
 		memcpy(&state(hdlc)->settings, &new_settings, size);
 		dev->hard_start_xmit = hdlc->xmit;
 		dev->hard_header = cisco_hard_header;
-		dev->hard_header_cache = NULL;
 		dev->type = ARPHRD_CISCO;
-		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
-		dev->addr_len = 0;
 		netif_dormant_on(dev);
 		return 0;
 	}
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index b45ab68..c6c3c75 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -1289,10 +1289,7 @@ static int fr_ioctl(struct net_device *dev, struct ifreq *ifr)
 		memcpy(&state(hdlc)->settings, &new_settings, size);
 
 		dev->hard_start_xmit = hdlc->xmit;
-		dev->hard_header = NULL;
 		dev->type = ARPHRD_FRAD;
-		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
-		dev->addr_len = 0;
 		return 0;
 
 	case IF_PROTO_FR_ADD_PVC:
diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index e9f7170..4591437 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -127,9 +127,7 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		if (result)
 			return result;
 		dev->hard_start_xmit = hdlc->xmit;
-		dev->hard_header = NULL;
 		dev->type = ARPHRD_PPP;
-		dev->addr_len = 0;
 		netif_dormant_off(dev);
 		return 0;
 	}
diff --git a/drivers/net/wan/hdlc_raw.c b/drivers/net/wan/hdlc_raw.c
index fe3cae5..e23bc66 100644
--- a/drivers/net/wan/hdlc_raw.c
+++ b/drivers/net/wan/hdlc_raw.c
@@ -88,10 +88,7 @@ static int raw_ioctl(struct net_device *dev, struct ifreq *ifr)
 			return result;
 		memcpy(hdlc->state, &new_settings, size);
 		dev->hard_start_xmit = hdlc->xmit;
-		dev->hard_header = NULL;
 		dev->type = ARPHRD_RAWHDLC;
-		dev->flags = IFF_POINTOPOINT | IFF_NOARP;
-		dev->addr_len = 0;
 		netif_dormant_off(dev);
 		return 0;
 	}
diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c
index e4bb9f8..cd7b22f 100644
--- a/drivers/net/wan/hdlc_x25.c
+++ b/drivers/net/wan/hdlc_x25.c
@@ -215,9 +215,7 @@ static int x25_ioctl(struct net_device *dev, struct ifreq *ifr)
 						   x25_rx, 0)) != 0)
 			return result;
 		dev->hard_start_xmit = x25_xmit;
-		dev->hard_header = NULL;
 		dev->type = ARPHRD_X25;
-		dev->addr_len = 0;
 		netif_dormant_off(dev);
 		return 0;
 	}

Comment 11 Anonymous Emailer 2007-03-02 15:38:27 UTC
Reply-To: khc@pm.waw.pl

David Miller <davem@davemloft.net> writes:

> I disagree, you can't leave dangling references to functions
> which are potentially inside of unloaded modules, as this code
> does.

All such pointers were thought to be initialized by all HDLC protocol
handlers before device activation, but they were actually used by the
hdlc* code, and this one doesn't seem to...

> Rather, HDLC Cisco should implement a proper protocol destructor
> method to clean up these function pointers.

No, it wouldn't work - hdlc_cisco doesn't use it at all, it's just
a victim. But now I think there may be other victims.

It seems the only way to become non-NULL is through ether_setup()
from hdlc_raw_eth.c (Ethernet framing over HDLC).

I think it's best to NULLify it and the like in hdlc.c
unconditionally, it's slow path and we don't need another useless
EXPORT_SYMBOL(s). It would fix all such problems forever.

Compile-tested only but it seems pretty obvious and of course I check
if the packets still flow after regular kernel upgrades (and I run
automatic tests checking all protos except X.25 from time to time as
well).

(the patch is in the next message).

Not sure if 2.6.21 material.
Comment 12 Anonymous Emailer 2007-03-02 15:43:30 UTC
Reply-To: davem@davemloft.net

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Sat, 03 Mar 2007 00:38:05 +0100

> Switching HDLC devices from Ethernet-framing mode caused stale ethernet
> function assignments within net_device.
> 
> Signed-off-by: Krzysztof Halasa <khc@pm.waw.pl>

This looks good to me, I think I'll apply it :-)

Thanks!

Comment 13 Chongfeng Hu 2007-03-03 23:03:52 UTC
Actually, I found another place where header_cache_update is dangling:

in drivers/media/dvb/dvb-core/dvb_net.c, in function static void dvb_net_setup
(struct net_device *dev), dev->hard_header_cache is initialized, but dev-
>header_cache_update is not:

    dev->open       = dvb_net_open;
    dev->stop       = dvb_net_stop;
    dev->hard_start_xmit    = dvb_net_tx;
    dev->get_stats      = dvb_net_get_stats;
    dev->set_multicast_list = dvb_net_set_multicast_list;
    dev->set_mac_address    = dvb_net_set_mac;
    dev->mtu        = 4096;
    dev->mc_count           = 0;
    dev->hard_header_cache  = NULL;
    dev->flags |= IFF_NOARP;

A third spot where hard_header_cache is dangling:

in drivers/net/skfp/skfddi.c, in function static int skfp_init_one(struct 
pci_dev *pdev, const struct pci_device_id *ent), dev->header_cache_update is 
updated, but dev->hard_header_cache is not:

    dev->irq = pdev->irq;
    dev->get_stats = &skfp_ctl_get_stats;
    dev->open = &skfp_open;
    dev->stop = &skfp_close;
    dev->hard_start_xmit = &skfp_send_pkt;
    dev->set_multicast_list = &skfp_ctl_set_multicast_list;
    dev->set_mac_address = &skfp_ctl_set_mac_address;
    dev->do_ioctl = &skfp_ioctl;
    dev->header_cache_update = NULL;    /* not supported */
Comment 14 Alan 2008-09-23 08:38:25 UTC
dvb case is already zeroed memory from kcalloc
skfddi likewise

At least as of 2.6.27rc

Note You need to log in before you can comment on or make changes to this bug.