Re: [PATCH] [IrDA] af_irda memory leak fixes
On 18/01/2008, Samuel Ortiz [EMAIL PROTECTED] wrote: Hi Dave, Here goes an IrDA patch against your latest net-2.6 tree. This patch fixes some af_irda memory leaks. It also checks for irias_new_obect() return value. Signed-off-by: Jesper Juhl [EMAIL PROTECTED] Signed-off-by: Samuel Ortiz [EMAIL PROTECTED] ... Looks good Samuel. Thank you for improving on my original suggested patch. Now let's hope someone queues it up for mainline :) -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] irda: return -ENOMEM upon failure to allocate new ias_obj
irias_new_object() can fail its memory allocation and will return NULL in
that case. I believe the proper thing to do is to catch this, free the
ias_opt that was allocated earlier but won't be used and then return
-ENOMEM.
There are assertions further on that check for a NULL ias_obj, but I think
it's a lot nicer to simply return -ENOMEM to the caller here where we know
a memory allocation failed, rather than hitting an assertion later.
note: I don't have any means of actually testing this, so it has been
compile tested only.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
af_irda.c |4
1 file changed, 4 insertions(+)
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index d5e4dd7..e33f0a5 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1881,6 +1881,10 @@ static int irda_setsockopt(struct socket *sock, int
level, int optname,
/* Create a new object */
ias_obj = irias_new_object(ias_opt-irda_class_name,
jiffies);
+ if (!ias_obj) {
+ kfree(ias_opt);
+ return -ENOMEM;
+ }
}
/* Do we have the attribute already ? */
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] irda: avoid potential memory leak in irda_setsockopt()
There are paths through the irda_setsockopt() function where we return and
may or may not have allocated a new ias_obj but in any case have not used
it for anything yet and we end up leaking memory.
As far as I can tell, in the case where we didn't allocate a new ias_ob
but simply were planning to use one already available then we should not
free it before returning. But when we have allocated a brand new ias_obj
but have not yet used it or put it on any lists etc and then return,
that's a memory leak.
There are two cases:
1)
switch (optname) {
case IRLMP_IAS_SET:
...
if(ias_obj == (struct ias_object *) NULL) {
/* Create a new object */
--[alloc]-- ias_obj = irias_new_object(ias_opt-irda_class_name,
jiffies);
...
switch(ias_opt-irda_attrib_type) {
case IAS_OCT_SEQ:
/* Check length */
if(ias_opt-attribute.irda_attrib_octet_seq.len
IAS_MAX_OCTET_STRING) {
kfree(ias_opt);
--[leak]--return -EINVAL;
...
}
irias_insert_object(ias_obj);
The allocated object isn't referenced at all until we get outside the
inner switch, so clearly we leak it (if we took the path that allocated it
that is).
2)
The second case is the same as the above, except it's the default: case in
the inner switch instead of case IAS_OCT_SEQ:
default :
kfree(ias_opt);
return -EINVAL;
The way I propose to fix this is with a new variable that keeps track of
whether or not we found an existing ias_obj to use or if we took the
allocation path, then use that variable to determine if we should free
ias_obj before returning from the function.
I'm not very intimate with this code, so if there's a better solution I'd
very much like to hear it. It's also entirely possible that someone with
more knowledge of this code can prove that these cases can't actually ever
happen - if that's the case then please let me know.
This patch is meant to be applied on top of
[PATCH 1/2] irda: return -ENOMEM upon failure to allocate new ias_obj
The Coverity checker gets credit for pointing its finger towards this.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
af_irda.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index e33f0a5..352e8a7 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1824,7 +1824,8 @@ static int irda_setsockopt(struct socket *sock, int
level, int optname,
struct irda_sock *self = irda_sk(sk);
struct irda_ias_set*ias_opt;
struct ias_object *ias_obj;
- struct ias_attrib * ias_attr; /* Attribute in IAS object */
+ struct ias_attrib *ias_attr; /* Attribute in IAS object */
+ int alloc_new_obj = 0;
int opt;
IRDA_DEBUG(2, %s(%p)\n, __FUNCTION__, self);
@@ -1885,6 +1886,7 @@ static int irda_setsockopt(struct socket *sock, int
level, int optname,
kfree(ias_opt);
return -ENOMEM;
}
+ alloc_new_obj = 1;
}
/* Do we have the attribute already ? */
@@ -1908,6 +1910,8 @@ static int irda_setsockopt(struct socket *sock, int
level, int optname,
if(ias_opt-attribute.irda_attrib_octet_seq.len
IAS_MAX_OCTET_STRING) {
kfree(ias_opt);
+ if (alloc_new_obj)
+ kfree(ias_obj);
return -EINVAL;
}
/* Add an octet sequence attribute */
@@ -1936,6 +1940,8 @@ static int irda_setsockopt(struct socket *sock, int
level, int optname,
break;
default :
kfree(ias_opt);
+ if (alloc_new_obj)
+ kfree(ias_obj);
return -EINVAL;
}
irias_insert_object(ias_obj);
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] forcedeth: fix MAC address detection on network card (regression in 2.6.23)
On 22/11/2007, Michael Pyne [EMAIL PROTECTED] wrote: On Wednesday 21 November 2007, Andrew Morton wrote: On Wed, 21 Nov 2007 15:34:52 -0800 Ayaz Abdulla [EMAIL PROTECTED] wrote: The solution is to get the OEM to update their BIOS (instead of integrating this patch) since the MCP61 specs indicate that the MAC Address should be in correct order from BIOS. By changing the feature DEV_HAS_CORRECT_MACADDR to all MCP61 boards, it could cause it to break on other OEM systems who have implemented it correctly. Getting an OEM to fix their BIOS isn't always a simple thing... Perhaps Michael's change should be enabled by a module parameter for those who happen to have the busted BIOS? I have contacted the motherboard vendor about this a couple of weeks ago per Ayaz's request and have received no response. I've also upgraded to the latest firmware for this motherboard and the bug remains. I think it would be ideal if there were a way to detect broken MCP61's (i.e. those with a Gigabyte MAC ID instead of the nVidia one) and only reverse the MAC address then. A module parameter would also work but then I'd need to remember to apply it. :) Hmm, MAC address makeups are not my strong point, but are the no rules describing the various parts of the address that could perhaps be used to infer programatically if the address seems to be reversed or not, and then use that detection logic for all boards that are known to potentially have the issue? A module parameter that overrules the automatic detection (for when it gets it wrong) would probably also be a good idea. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix memory leak in discard case of sctp_sf_abort_violation()
From: Jesper Juhl [EMAIL PROTECTED]
In net/sctp/sm_statefuns.c::sctp_sf_abort_violation() we may leak
the storage allocated for 'abort' by returning from the function
without using or freeing it. This happens in case
sctp_auth_recv_cid(SCTP_CID_ABORT, asoc) is true and we jump to
the 'discard' label.
Spotted by the Coverity checker.
The simple fix is to simply move the creation of the abort chunk
to after the possible jump to the 'discard' label. This way we don't
even have to allocate the memory at all in the problem case.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
sm_statefuns.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index f01b408..4c5c5e7 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4064,11 +4064,6 @@ static sctp_disposition_t sctp_sf_abort_violation(
struct sctp_chunk *chunk = arg;
struct sctp_chunk *abort = NULL;
- /* Make the abort chunk. */
- abort = sctp_make_abort_violation(asoc, chunk, payload, paylen);
- if (!abort)
- goto nomem;
-
/* SCTP-AUTH, Section 6.3:
*It should be noted that if the receiver wants to tear
*down an association in an authenticated way only, the
@@ -4083,6 +4078,11 @@ static sctp_disposition_t sctp_sf_abort_violation(
if (sctp_auth_recv_cid(SCTP_CID_ABORT, asoc))
goto discard;
+ /* Make the abort chunk. */
+ abort = sctp_make_abort_violation(asoc, chunk, payload, paylen);
+ if (!abort)
+ goto nomem;
+
if (asoc) {
sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/30] net: Don't do pointless kmalloc return value casts in zd1211 driver
On 30/08/2007, Daniel Drake [EMAIL PROTECTED] wrote: Jesper Juhl wrote: Since kmalloc() returns a void pointer there is no reason to cast its return value. This patch also removes a pointless initialization of a variable. NAK: adds a sparse warning zd_chip.c:116:15: warning: implicit cast to nocast type Ok, I must admit I didn't check with sparse since it seemed pointless - we usually never cast void pointers to other pointer types, specifically because the C language nicely guarantees that the right thing will happen without the cast. Sometimes we have to cast them to integer types, su sure we need the cast there. But what on earth makes a zd_addr_t * so special that we have to explicitly cast a void * to that type? I see it's defined as typedef u32 __nocast zd_addr_t; in drivers/net/wireless/zd1211rw/zd_types.h , but why the __nocast ? What would be wrong in applying my patch that removes the cast of the kmalloc() return value and then also remove the __nocast here? -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Don't needlessly initialize variable to NULL in zd_chip (was: Re: [PATCH 13/30] net: Don't do pointless kmalloc return value casts in zd1211 driver)
On Friday 31 August 2007 00:19:53 Joe Perches wrote:
On Thu, 2007-08-30 at 22:20 +0200, Jesper Juhl wrote:
Ok, I must admit I didn't check with sparse since it seemed pointless
- we usually never cast void pointers to other pointer types,
specifically because the C language nicely guarantees that the right
thing will happen without the cast. Sometimes we have to cast them to
integer types, su sure we need the cast there. But what on earth
makes a zd_addr_t * so special that we have to explicitly cast a
void * to that type?
http://marc.info/?l=linux-netdevm=117113743902549w=1
Thank you for that link Joe.
I'm not sure I agree with the __nocast, but I respect that this is
the maintainers choice.
But, I still think the first chunk of the patch that removes the
initial variable initialization makes sense.
Initializing the variable to NULL is pointless since it'll never be
used before the kmalloc() call. So here's a revised patch that just
gets rid of that little detail.
No need to initialize to NULL when variable is never used before
it's assigned the return value of a kmalloc() call.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
diff --git a/drivers/net/wireless/zd1211rw/zd_chip.c
b/drivers/net/wireless/zd1211rw/zd_chip.c
index c39f198..30872fe 100644
--- a/drivers/net/wireless/zd1211rw/zd_chip.c
+++ b/drivers/net/wireless/zd1211rw/zd_chip.c
@@ -106,7 +106,7 @@ int zd_ioread32v_locked(struct zd_chip *chip, u32 *values,
const zd_addr_t *addr
{
int r;
int i;
- zd_addr_t *a16 = (zd_addr_t *)NULL;
+ zd_addr_t *a16;
u16 *v16;
unsigned int count16;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Don't needlessly initialize variable to NULL in zd_chip (was: Re: [PATCH 13/30] net: Don't do pointless kmalloc return value casts in zd1211 driver)
On 31/08/2007, Randy Dunlap [EMAIL PROTECTED] wrote: ... BTW: http://marc.info/?l=linux-wirelessm=118831813500769w=2 ... Heh, thanks Randy. All too often patches get missed since I don't happen to include the right magic person to Cc. So I generally take a better to have one Cc too many than miss one approach when sending patches - otherwise I just end up resending it several times and eventually have to bother Andrew to move it through -mm. I see the point of people not getting things twice, but too many patches slip through the cracks already (and not just my patches, that's a general problem) so I'd rather inconvenience a few people with one extra email than missing the proper maintainer by not Cc'ing the right list.Picking the right list/set of people to Cc is hard! (whoops, mistakenly didn't do a reply-to-all initially; sorry Randy, now you'll get this mail twice ;) -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/30] net: Kill some unneeded allocation return value casts in libertas
kmalloc() and friends return void*, no need to cast it. Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- drivers/net/wireless/libertas/debugfs.c |2 +- drivers/net/wireless/libertas/ethtool.c |3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c index 715cbda..6ade63e 100644 --- a/drivers/net/wireless/libertas/debugfs.c +++ b/drivers/net/wireless/libertas/debugfs.c @@ -1839,7 +1839,7 @@ static ssize_t wlan_debugfs_write(struct file *f, const char __user *buf, char *p2; struct debug_data *d = (struct debug_data *)f-private_data; - pdata = (char *)kmalloc(cnt, GFP_KERNEL); + pdata = kmalloc(cnt, GFP_KERNEL); if (pdata == NULL) return 0; diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c index 96f1974..7dad493 100644 --- a/drivers/net/wireless/libertas/ethtool.c +++ b/drivers/net/wireless/libertas/ethtool.c @@ -60,8 +60,7 @@ static int libertas_ethtool_get_eeprom(struct net_device *dev, // mutex_lock(priv-mutex); - adapter-prdeeprom = - (char *)kmalloc(eeprom-len+sizeof(regctrl), GFP_KERNEL); + adapter-prdeeprom = kmalloc(eeprom-len+sizeof(regctrl), GFP_KERNEL); if (!adapter-prdeeprom) return -ENOMEM; memcpy(adapter-prdeeprom, regctrl, sizeof(regctrl)); -- 1.5.2.2 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/30] net: Avoid pointless allocation casts in BSD compression module
The general kernel memory allocation functions return void pointers
and there is no need to cast their return values.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
drivers/net/bsd_comp.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bsd_comp.c b/drivers/net/bsd_comp.c
index 202d4a4..88edb98 100644
--- a/drivers/net/bsd_comp.c
+++ b/drivers/net/bsd_comp.c
@@ -406,8 +406,7 @@ static void *bsd_alloc (unsigned char *options, int
opt_len, int decomp)
* Allocate space for the dictionary. This may be more than one page in
* length.
*/
-db-dict = (struct bsd_dict *) vmalloc (hsize *
- sizeof (struct bsd_dict));
+db-dict = vmalloc(hsize * sizeof(struct bsd_dict));
if (!db-dict)
{
bsd_free (db);
@@ -426,8 +425,7 @@ static void *bsd_alloc (unsigned char *options, int
opt_len, int decomp)
*/
else
{
-db-lens = (unsigned short *) vmalloc ((maxmaxcode + 1) *
- sizeof (db-lens[0]));
+db-lens = vmalloc((maxmaxcode + 1) * sizeof(db-lens[0]));
if (!db-lens)
{
bsd_free (db);
--
1.5.2.2
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xen-netfront: Avoid deref'ing skbafter it is potentially freed.
On Monday 13 August 2007 21:54:37 Jeremy Fitzhardinge wrote: xennet_tx_bug_gc can free the skb before we use it, so make sure we don't. Jeff, this is -rc material. Signed-off-by: Keir Fraser [EMAIL PROTECTED] Signed-off-by: Jeremy Fitzhardinge [EMAIL PROTECTED] Cc: Jeff Garzik [EMAIL PROTECTED] diff -r 8bfc43f6d1b0 drivers/net/xen-netfront.c --- a/drivers/net/xen-netfront.c Tue Aug 07 14:26:30 2007 -0700 +++ b/drivers/net/xen-netfront.c Mon Aug 13 09:39:15 2007 -0700 @@ -566,15 +566,16 @@ static int xennet_start_xmit(struct sk_b if (notify) notify_remote_via_irq(np-netdev-irq); + np-stats.tx_bytes += skb-len; + np-stats.tx_packets++; + + /* Note: It is not safe to access skb after xennet_tx_buf_gc()! */ xennet_tx_buf_gc(dev); if (!netfront_tx_slot_available(np)) netif_stop_queue(dev); spin_unlock_irq(np-tx_lock); - - np-stats.tx_bytes += skb-len; - np-stats.tx_packets++; return 0; This moves the updating of both tx_bytes and tx_packets inside the spinlock, but as far as I can see we only _really_ need to move the tx_bytes update. Considering that we generally want to do as little work as possible while holding a lock, wouldn't the following be slightly better? Signed-off-by: Keir Fraser [EMAIL PROTECTED] Signed-off-by: Jeremy Fitzhardinge [EMAIL PROTECTED] Cc: Jeff Garzik [EMAIL PROTECTED] Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- drivers/net/xen-netfront.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 489f69c..640e270 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -566,6 +566,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) if (notify) notify_remote_via_irq(np-netdev-irq); + np-stats.tx_bytes += skb-len; + + /* Note: It is not safe to access skb after xennet_tx_buf_gc()! */ xennet_tx_buf_gc(dev); if (!netfront_tx_slot_available(np)) @@ -573,7 +576,6 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) spin_unlock_irq(np-tx_lock); - np-stats.tx_bytes += skb-len; np-stats.tx_packets++; return 0; - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6][RESEND] Avoid possible NULL pointer deref in 3c359 driver
(Resending old patch originally submitted at 1/7-2007 02:19, 04-Aug-2007 20:31)
In xl_freemem(), if dev_if is NULL, the line
struct xl_private *xl_priv =(struct xl_private *)dev-priv;
will cause a NULL pointer dereference. However, if we move
that assignment below the 'if' statement that tests for a NULL
'dev', then that NULL deref can never happen.
It never hurts to be safe :-)
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
diff --git a/drivers/net/tokenring/3c359.c b/drivers/net/tokenring/3c359.c
index e22a3f5..671f4da 100644
--- a/drivers/net/tokenring/3c359.c
+++ b/drivers/net/tokenring/3c359.c
@@ -1044,15 +1044,17 @@ static void xl_freemem(struct net_device *dev)
static irqreturn_t xl_interrupt(int irq, void *dev_id)
{
struct net_device *dev = (struct net_device *)dev_id;
- struct xl_private *xl_priv =(struct xl_private *)dev-priv;
- u8 __iomem * xl_mmio = xl_priv-xl_mmio ;
- u16 intstatus, macstatus ;
+ struct xl_private *xl_priv;
+ u8 __iomem * xl_mmio;
+ u16 intstatus, macstatus;
if (!dev) {
- printk(KERN_WARNING Device structure dead, aaa !\n) ;
+ printk(KERN_WARNING 3c359: Device structure dead, aaa!\n);
return IRQ_NONE;
}
+ xl_priv = (struct xl_private *)dev-priv;
+ xl_mmio = xl_priv-xl_mmio;
intstatus = readw(xl_mmio + MMIO_INTSTATUS) ;
if (!(intstatus 1)) /* We didn't generate the interrupt */
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel oops report
(this one looks like it should go to netdev as well - added to Cc) On 10/08/07, Sinisa Segvic [EMAIL PROTECTED] wrote: Hi, I've just got a kernel oops. http://lxr.linux.no/source/Documentation/oops-tracing.txt seems to suggest that oops reports are welcome at this address. Cheers, Sinisa $ uname -a Linux PCs129.EMT.tugraz.at 2.6.20-16-386 #2 Thu Jun 7 20:16:13 UTC 2007 i686 GNU/Linux [326735.692443] BUG: unable to handle kernel NULL pointer dereference at virtual address 001b [326735.692454] printing eip: [326735.692457] c015f581 [326735.692458] *pde = [326735.692463] Oops: [#1] [326735.692466] Modules linked in: nls_cp437 isofs udf binfmt_misc rfcomm l2cap bluetooth nfs lockd sunrpc xt_limit xt_tcpudp iptable_mangle ipt_LOG ipt_MASQUERADE nf_nat ipt_TOS ipt_REJECT nf_conntrack_irc nf_conntrack_ftp nf_conntrack_ipv4 xt_state nf_conntrack nfnetlink iptable_filter ip_tables x_tables ppdev radeon drm cpufreq_userspace cpufreq_stats cpufreq_powersave cpufreq_ondemand freq_table cpufreq_conservative tc1100_wmi pcc_acpi dev_acpi sony_acpi video sbs i2c_ec dock button battery container ac asus_acpi backlight ipv6 fuse lp snd_cmipci gameport snd_pcm_oss snd_mixer_oss snd_pcm snd_page_alloc snd_opl3_lib snd_hwdep snd_mpu401_uart snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi snd_seq_midi_event i2c_viapro parport_pc parport pcspkr psmouse i2c_core snd_seq snd_timer snd_seq_device rtc via_ircc snd irda soundcore crc_ccitt serio_raw via_agp agpgart shpchp pci_hotplug af_packet evdev tsdev ext3 jbd mbcache ide_cd cdrom ide_disk ata_generic libata scsi_mod via82cxxx 8139too floppy 8139cp mii generic ohci1394 ieee1394 ehci_hcd uhci_hcd usbcore raid10 raid456 xor raid1 raid0 multipath linear md_mod thermal processor fan dm_mod fbcon tileblit font bitblit softcursor vesafb capability commoncap [326735.692550] CPU:0 [326735.692552] EIP:0060:[c015f581]Not tainted VLI [326735.692553] EFLAGS: 00210082 (2.6.20-16-386 #2) [326735.692565] EIP is at kfree+0x41/0x80 [326735.692568] eax: 4080 ebx: 001b ecx: dfffb3c4 edx: c11ce340 [326735.692572] esi: ce71ac00 edi: 00200286 ebp: cb2d64a0 esp: d0e4bdd8 [326735.692575] ds: 007b es: 007b ss: 0068 [326735.692579] Process firefox-bin (pid: 6763, ti=d0e4a000 task=cf56c590 task.ti=d0e4a000) [326735.692582] Stack: cb2d64a0 d0e4bedc d0e4bea0 c02701f8 0020 c02d0c4d c026d39d [326735.692589]d032bc80 d032bdc0 cb2d64d0 d032bcd4 d0e4bea0 cf12d8c0 0020 0001 [326735.692596]0001 ffa1 d0e4be80 1302 [326735.692601] Call Trace: [326735.692608] [c02701f8] kfree_skbmem+0x8/0x80 [326735.692619] [c02d0c4d] unix_stream_recvmsg+0x1ad/0x540 [326735.692627] [c026d39d] sock_alloc_send_skb+0x16d/0x1c0 [326735.692659] [c011fad6] current_fs_time+0x46/0x50 [326735.692670] [c026a37f] sock_aio_read+0x11f/0x130 [326735.692679] [c0168f2b] pipe_write+0x22b/0x470 [326735.692706] [c0162ce5] do_sync_read+0xd5/0x120 [326735.692729] [c012ded0] autoremove_wake_function+0x0/0x50 [326735.692746] [c0130a72] hrtimer_run_queues+0xf2/0x120 [326735.692754] [c02cfb79] unix_ioctl+0x89/0xc0 [326735.692771] [c016375c] vfs_read+0x17c/0x190 [326735.692782] [c0163b11] sys_read+0x41/0x70 [326735.692793] [c0102fc0] sysenter_past_esp+0x69/0xa9 [326735.692817] === [326735.692819] Code: 05 00 00 00 00 8d 96 00 00 00 40 89 c7 c1 ea 0c c1 e2 05 03 15 80 ba 44 c0 8b 02 f6 c4 40 75 2b 8b 02 84 c0 79 37 8b 4a 18 8b 19 8b 03 3b 43 04 73 1e 89 74 83 10 83 c0 01 89 03 89 f8 50 9d 8d [326735.692843] EIP: [c015f581] kfree+0x41/0x80 SS:ESP 0068:d0e4bdd8 [326735.692849] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] make atomic_t volatile on all architectures
On 09/08/2007, Chris Snook [EMAIL PROTECTED] wrote: From: Chris Snook [EMAIL PROTECTED] Some architectures currently do not declare the contents of an atomic_t to be volatile. This causes confusion since atomic_read() might not actually read anything if an optimizing compiler re-uses a value stored in a register, which can break code that loops until something external changes the value of an atomic_t. Avoiding such bugs requires using barrier(), which causes re-loads of all registers used in the loop, thus hurting performance instead of helping it, particularly on architectures where it's unnecessary. Since we generally want to re-read the contents of an atomic variable on every access anyway, let's standardize the behavior across all architectures and avoid the performance and correctness problems of requiring the use of barrier() in loops that expect atomic_t variables to change externally. This is relevant even on non-smp architectures, since drivers may use atomic operations in interrupt handlers. Signed-off-by: Chris Snook [EMAIL PROTECTED] Hmm, I thought we were trying to move away from volatile since it is very weakly defined and towards explicit barriers and locks... Points to -- Documentation/volatile-considered-harmful.txt -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] make atomic_t volatile on all architectures
On 09/08/2007, Chris Snook [EMAIL PROTECTED] wrote: Jesper Juhl wrote: On 09/08/2007, Chris Snook [EMAIL PROTECTED] wrote: From: Chris Snook [EMAIL PROTECTED] Some architectures currently do not declare the contents of an atomic_t to be volatile. This causes confusion since atomic_read() might not actually read anything if an optimizing compiler re-uses a value stored in a register, which can break code that loops until something external changes the value of an atomic_t. Avoiding such bugs requires using barrier(), which causes re-loads of all registers used in the loop, thus hurting performance instead of helping it, particularly on architectures where it's unnecessary. Since we generally want to re-read the contents of an atomic variable on every access anyway, let's standardize the behavior across all architectures and avoid the performance and correctness problems of requiring the use of barrier() in loops that expect atomic_t variables to change externally. This is relevant even on non-smp architectures, since drivers may use atomic operations in interrupt handlers. Signed-off-by: Chris Snook [EMAIL PROTECTED] Hmm, I thought we were trying to move away from volatile since it is very weakly defined and towards explicit barriers and locks... Points to -- Documentation/volatile-considered-harmful.txt This is a special case. I had a feeling it might be. Usually, the use of volatile is just lazy. In this case, it's probably necessary on at least some architectures, so we can't remove it everywhere unless we want to rewrite atomic.h completely in inline assembler for several architectures, and painstakingly verify all that work. Sounds quite unpleasant and actively harmful - keeping stuff in plain readable C makes it easier to handle by most people. I would hope it's obvious that having consistent behavior on all architectures, or even at all compiler optimization levels within an architecture, can be agreed to be good. Agreed, consistency is good. Additionally, atomic_t variables are a rare exception, in that we pretty much always want to work with the latest value in RAM on every access. Making this atomic will allow us to remove a bunch of barriers which do nothing but slow things down on most architectures. I believe you meant to say volatile and not atomic above. But yes, if volatile is sufficiently defined to ensure it does what we want in this case and using it means we can actually speed up the kernel, then it does indeed sound reasonable. Especially since it's confined to the implementation of atomic_t which most people shouldn't be looking at anyway (but simply use) and when using an atomic_t it's pretty reasonable to expect that it doesn't need any additional locking or barriers since it's supposed to be *atomic*. I agree that the use of atomic_t in .c files is generally bad, but in certain special cases, hiding it inside defined data types may be worth the slight impurity, just as we sometimes tolerate lines longer than 80 columns when fixing it makes things unreadable. Can't argue with that. It seems you've thought it through. I just wanted to be sure that this approach was actually the one that makes the most sense, and you've convinced me of that :-) -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][RESEND] Avoid possible NULL pointer deref in 3c359 driver
(Resending old patch originally submitted at 1/7-2007 02:19)
In xl_freemem(), if dev_if is NULL, the line
struct xl_private *xl_priv =(struct xl_private *)dev-priv;
will cause a NULL pointer dereference. However, if we move
that assignment below the 'if' statement that tests for a NULL
'dev', then that NULL deref can never happen.
It never hurts to be safe :-)
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
diff --git a/drivers/net/tokenring/3c359.c b/drivers/net/tokenring/3c359.c
index e22a3f5..671f4da 100644
--- a/drivers/net/tokenring/3c359.c
+++ b/drivers/net/tokenring/3c359.c
@@ -1044,15 +1044,17 @@ static void xl_freemem(struct net_device *dev)
static irqreturn_t xl_interrupt(int irq, void *dev_id)
{
struct net_device *dev = (struct net_device *)dev_id;
- struct xl_private *xl_priv =(struct xl_private *)dev-priv;
- u8 __iomem * xl_mmio = xl_priv-xl_mmio ;
- u16 intstatus, macstatus ;
+ struct xl_private *xl_priv;
+ u8 __iomem * xl_mmio;
+ u16 intstatus, macstatus;
if (!dev) {
- printk(KERN_WARNING Device structure dead, aaa !\n) ;
+ printk(KERN_WARNING 3c359: Device structure dead, aaa!\n);
return IRQ_NONE;
}
+ xl_priv = (struct xl_private *)dev-priv;
+ xl_mmio = xl_priv-xl_mmio;
intstatus = readw(xl_mmio + MMIO_INTSTATUS) ;
if (!(intstatus 1)) /* We didn't generate the interrupt */
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB Pegasus driver - avoid a potential NULL pointer dereference.
Hello,
This patch makes sure we don't dereference a NULL pointer in
drivers/net/usb/pegasus.c::write_bulk_callback() in the initial
struct net_device *net = pegasus-net; assignment.
The existing code checks if 'pegasus' is NULL and bails out if
it is, so we better not touch that pointer until after that check.
Please consider merging.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
drivers/net/usb/pegasus.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index a05fd97..04cba6b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -768,11 +768,13 @@ done:
static void write_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb-context;
- struct net_device *net = pegasus-net;
+ struct net_device *net;
if (!pegasus)
return;
+ net = pegasus-net;
+
if (!netif_device_present(net) || !netif_running(net))
return;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB Pegasus driver - avoid a potential NULL pointer dereference.
On 29/07/07, Satyam Sharma [EMAIL PROTECTED] wrote:
Hi,
On 7/29/07, Jesper Juhl [EMAIL PROTECTED] wrote:
Hello,
This patch makes sure we don't dereference a NULL pointer in
drivers/net/usb/pegasus.c::write_bulk_callback() in the initial
struct net_device *net = pegasus-net; assignment.
The existing code checks if 'pegasus' is NULL and bails out if
it is, so we better not touch that pointer until after that check.
[...]
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index a05fd97..04cba6b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -768,11 +768,13 @@ done:
static void write_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb-context;
- struct net_device *net = pegasus-net;
+ struct net_device *net;
if (!pegasus)
return;
+ net = pegasus-net;
+
if (!netif_device_present(net) || !netif_running(net))
return;
Is it really possible that we're called into this function with
urb-context == NULL? If not, I'd suggest let's just get rid of
the check itself, instead.
I'm not sure. I am not very familiar with this code. I just figured
that moving the assignment is potentially a little safer and it is
certainly no worse than the current code, so that's a safe and
potentially benneficial change. Removing the check may be safe but I'm
not certain enough to make that call...
--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][30/37] Clean up duplicate includes in net/netfilter/
On 24/07/07, Patrick McHardy [EMAIL PROTECTED] wrote: Jesper Juhl wrote: This patch cleans up duplicate includes in net/netfilter/ I've queued this one and the bridge-netfilter patch (27/37), thanks Jesper. Thanks for the feedback Patrick. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][26/37] Clean up duplicate includes in net/atm/
Hi, This patch cleans up duplicate includes in net/atm/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/atm/lec.c b/net/atm/lec.c index 2770fb4..59d5aa3 100644 --- a/net/atm/lec.c +++ b/net/atm/lec.c @@ -21,7 +21,6 @@ #include net/dst.h #include linux/proc_fs.h #include linux/spinlock.h -#include linux/proc_fs.h #include linux/seq_file.h /* TokenRing if needed */ - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][27/37] Clean up duplicate includes in net/bridge/
Hi, This patch cleans up duplicate includes in net/bridge/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/bridge/netfilter/ebt_log.c b/net/bridge/netfilter/ebt_log.c index 031bfa4..89d0d59 100644 --- a/net/bridge/netfilter/ebt_log.c +++ b/net/bridge/netfilter/ebt_log.c @@ -9,7 +9,6 @@ * */ -#include linux/in.h #include linux/netfilter_bridge/ebtables.h #include linux/netfilter_bridge/ebt_log.h #include linux/netfilter.h diff --git a/net/bridge/netfilter/ebt_ulog.c b/net/bridge/netfilter/ebt_ulog.c index 9411db6..a31402a 100644 --- a/net/bridge/netfilter/ebt_ulog.c +++ b/net/bridge/netfilter/ebt_ulog.c @@ -36,7 +36,6 @@ #include linux/timer.h #include linux/netlink.h #include linux/netdevice.h -#include linux/module.h #include linux/netfilter_bridge/ebtables.h #include linux/netfilter_bridge/ebt_ulog.h #include net/sock.h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][29/37] Clean up duplicate includes in net/ipv6/
Hi, This patch cleans up duplicate includes in net/ipv6/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index d67fb1e..db50114 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -56,7 +56,6 @@ #include net/inet_ecn.h #include net/protocol.h #include net/xfrm.h -#include net/addrconf.h #include net/snmp.h #include net/dsfield.h #include net/timewait_sock.h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][31/37] Clean up duplicate includes in net/sched/
Hi, This patch cleans up duplicate includes in net/sched/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/sched/act_police.c b/net/sched/act_police.c index bf90e60..6085be5 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -16,7 +16,6 @@ #include linux/string.h #include linux/errno.h #include linux/skbuff.h -#include linux/module.h #include linux/rtnetlink.h #include linux/init.h #include net/act_api.h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][30/37] Clean up duplicate includes in net/netfilter/
Hi, This patch cleans up duplicate includes in net/netfilter/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c index 87ad3cc..eb3fe74 100644 --- a/net/netfilter/nf_conntrack_proto_tcp.c +++ b/net/netfilter/nf_conntrack_proto_tcp.c @@ -8,7 +8,6 @@ #include linux/types.h #include linux/timer.h -#include linux/netfilter.h #include linux/module.h #include linux/in.h #include linux/tcp.h diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c index 13d94a0..2a2fd1a 100644 --- a/net/netfilter/nf_conntrack_proto_udp.c +++ b/net/netfilter/nf_conntrack_proto_udp.c @@ -9,7 +9,6 @@ #include linux/types.h #include linux/timer.h #include linux/module.h -#include linux/netfilter.h #include linux/udp.h #include linux/seq_file.h #include linux/skbuff.h diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c index 93e747b..b906b41 100644 --- a/net/netfilter/nf_conntrack_proto_udplite.c +++ b/net/netfilter/nf_conntrack_proto_udplite.c @@ -10,7 +10,6 @@ #include linux/types.h #include linux/timer.h #include linux/module.h -#include linux/netfilter.h #include linux/udp.h #include linux/seq_file.h #include linux/skbuff.h diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c index f47cab7..a4bab04 100644 --- a/net/netfilter/xt_physdev.c +++ b/net/netfilter/xt_physdev.c @@ -13,7 +13,6 @@ #include linux/netfilter_bridge.h #include linux/netfilter/xt_physdev.h #include linux/netfilter/x_tables.h -#include linux/netfilter_bridge.h MODULE_LICENSE(GPL); MODULE_AUTHOR(Bart De Schuymer [EMAIL PROTECTED]); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][34/37] Clean up duplicate includes in net/xfrm/
Hi, This patch cleans up duplicate includes in net/xfrm/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index c3a4b0a..e67489a 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -23,10 +23,9 @@ #include linux/netfilter.h #include linux/module.h #include linux/cache.h +#include linux/audit.h #include net/xfrm.h #include net/ip.h -#include linux/audit.h -#include linux/cache.h #include xfrm_hash.h diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 38f90ca..b2552b1 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -19,9 +19,8 @@ #include linux/ipsec.h #include linux/module.h #include linux/cache.h -#include asm/uaccess.h #include linux/audit.h -#include linux/cache.h +#include asm/uaccess.h #include xfrm_hash.h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][33/37] Clean up duplicate includes in net/tipc/
Hi, This patch cleans up duplicate includes in net/tipc/ Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- diff --git a/net/tipc/port.c b/net/tipc/port.c index 5d2b9ce..7608815 100644 --- a/net/tipc/port.c +++ b/net/tipc/port.c @@ -41,7 +41,6 @@ #include addr.h #include link.h #include node.h -#include port.h #include name_table.h #include user_reg.h #include msg.h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -mm 4/9] netconsole: Add some useful tips to documentation
On 11/07/07, Satyam Sharma [EMAIL PROTECTED] wrote: On Tue, 10 Jul 2007, Joel Becker wrote: On Wed, Jul 11, 2007 at 03:40:22AM +0530, Satyam Sharma wrote: IMHO something that mentions /proc/sys/kernel/printk would be better. You don't need to have built with SysRq support for that, it's clearly more flexible than the ignore_loglevel option and wouldn't require a reboot either. I'll send out an updated patch shortly. Why not dmesg -n? We've been using that for years. Or is there some extra change in /proc/sys/kernel/printk? Yes, dmesg -n sounds the most straightforward. There are multiple ways of course, for some reason I've always used /proc/sys/kernel/printk for this (which has the extra feature that it accepts values greater than 8 too :-) Why not mention all the various methods, dmesg -n, /proc/sys/kernel/printk, SysRq, ignore_loglevel ?? -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -mm 4/9] netconsole: Add some useful tips to documentation
On Tuesday 10 July 2007 11:41:43 Matt Mackall wrote: On Tue, Jul 10, 2007 at 02:49:41PM +0530, Satyam Sharma wrote: From: Satyam Sharma [EMAIL PROTECTED] [4/9] netconsole: Add some useful tips to documentation Add some useful general-purpose tips. Signed-off-by: Satyam Sharma [EMAIL PROTECTED] Cc: Keiichi Kii [EMAIL PROTECTED] Acked-by: Matt Mackall [EMAIL PROTECTED] As long as we're on the subject, I've been meaning to add a note telling people to set their console log level to something useful, as having that set too low is the most common problem people encounter. How about this? From: Satyam Sharma [EMAIL PROTECTED] Add some useful general-purpose tips. Signed-off-by: Satyam Sharma [EMAIL PROTECTED] Cc: Keiichi Kii [EMAIL PROTECTED] Acked-by: Matt Mackall [EMAIL PROTECTED] Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- Documentation/networking/netconsole.txt | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/Documentation/networking/netconsole.txt b/Documentation/networking/netconsole.txt index 1caa6c7..ffd5058 100644 --- a/Documentation/networking/netconsole.txt +++ b/Documentation/networking/netconsole.txt @@ -44,11 +44,29 @@ WARNING: the default target ethernet setting uses the broadcast ethernet address to send packets, which can cause increased load on other systems on the same ethernet segment. +TIP: some LAN switches may be configured to suppress ethernet broadcasts +so it is advised to explicitly specify the remote agents' MAC addresses +from the config parameters passed to netconsole. + +TIP: to find out the MAC address of, say, 10.0.0.2, you may try using: + + ping -c 1 10.0.0.2 ; /sbin/arp -n | grep 10.0.0.2 + +TIP: in case the remote logging agent is on a separate LAN subnet than +the sender, it is suggested to try specifying the MAC address of the +default gateway (you may use /sbin/route -n to find it out) as the +remote MAC address instead. + NOTE: the network device (eth1 in the above case) can run any kind of other network traffic, netconsole is not intrusive. Netconsole might cause slight delays in other traffic if the volume of kernel messages is high, but should have no other impact. +Some people forget to raise the kernels log_level to an +appropriate level and thus don't see all the kernel log messages they +expect. You can add the kernel boot option ignore_loglevel to see all +messages or you can use SysRq to set the log level to a specific value. + Netconsole was designed to be as instantaneous as possible, to enable the logging of even the most critical kernel bugs. It works from IRQ contexts as well, and does not enable interrupts while - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/net/chelsio/my3126.c: inconsequent NULL checking
On 07/12/06, Adrian Bunk [EMAIL PROTECTED] wrote:
The Coverity checker spotted the following inconsequent NULL checking
introduced by commit f1d3d38af75789f1b82969b83b69cab540609789:
-- snip --
...
static struct cphy *my3126_phy_create(adapter_t *adapter,
int phy_addr, struct mdio_ops *mdio_ops)
{
struct cphy *cphy = kzalloc(sizeof (*cphy), GFP_KERNEL);
if (cphy)
cphy_init(cphy, adapter, phy_addr, my3126_ops, mdio_ops);
INIT_WORK(cphy-phy_update, my3216_poll, cphy);
cphy-bmsr = 0;
return (cphy);
}
...
-- snip --
It doesn't make sense to first check whether cphy is NULL and
dereference it unconditionally later.
How about simply changing
if (cphy)
cphy_init(cphy, adapter, phy_addr, my3126_ops, mdio_ops);
into
if (!cphy)
return NULL;
callers need to be able to handle that ofcourse, but I haven't checked that yet.
--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
IPv4: ip_options_compile() how can we avoid blowing up on a NULL skb???
Hi,
In net/ipv4/ip_options.c::ip_options_compile() we have the following
code at the start of the function :
int ip_options_compile(struct ip_options * opt, struct sk_buff * skb)
{
int l;
unsigned char * iph;
unsigned char * optptr;
int optlen;
unsigned char * pp_ptr = NULL;
struct rtable *rt = skb ? (struct rtable*)skb-dst : NULL;
if (!opt) {
opt = (IPCB(skb)-opt);
iph = skb-nh.raw;
opt-optlen = ((struct iphdr *)iph)-ihl*4 -
sizeof(struct iphdr);
optptr = iph + sizeof(struct iphdr);
opt-is_data = 0;
} else {
optptr = opt-is_data ? opt-__data : (unsigned
char*)(skb-nh.iph[1]);
iph = optptr - sizeof(struct iphdr);
}
...
I don't see how that avoids blowing up if we get passed a NULL skb.
From the line
struct rtable *rt = skb ? (struct rtable*)skb-dst : NULL;
it is clear that we /may/ get passed a null skb.
Then a bit further down in the if (!opt) bit we dereference 'skb' :
opt = (IPCB(skb)-opt);
and we also may dereference it in the else part :
optptr = opt-is_data ? opt-__data : (unsigned char*)(skb-nh.iph[1]);
So if 'skb' is NULL, the only route I see that doesn't cause a NULL
pointer deref is if (opt != NULL) and at the same time
(opt-is_data != NULL) . Is that guaranteed in any way? If now,
how come we don't blow up regularly?
--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New laptop - problems with linux
On 08/11/06, Stephen Clark [EMAIL PROTECTED] wrote: Stephen Hemminger wrote: On Wed, 08 Nov 2006 14:39:53 -0500 Stephen Clark [EMAIL PROTECTED] wrote: 03:00.0 Network controller: Intel Corporation PRO/Wireless 3945ABG Network Connection (rev 02) Subsystem: Intel Corporation Unknown device 1000 Use the ipw3945 driver and binary regulatory daemon from: http://ipw3945.sourceforge.net/#downloads Thanks I have that working - I am now struggling with the disk being slower than molasses ( high priority, 1.xx mb/sec ) and the integrated realtek ethernet ( low prioirty - since I got the wifi working ). As Stephen Hemminger wrote, getting your realtek card working should be a simple matter of either backporting the recent change that adds its PCI ID or simply run a 2.6.19-rc5 kernel that includes it. 05:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8169SC Gigabit Ethernet (rev 10) Subsystem: ASUSTeK Computer Inc. Unknown device 1345 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B- Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Latency: 64 (8000ns min, 16000ns max), Cache Line Size: 32 bytes Interrupt: pin A routed to IRQ 11 Region 0: I/O ports at d800 [size=256] Region 1: Memory at fe8ffc00 (32-bit, non-prefetchable) [size=256] Expansion ROM at 8000 [disabled] [size=128K] Capabilities: [dc] Power Management version 2 Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0-,D1+,D2+,D3hot+,D3cold+) Status: D0 PME-Enable- DSel=0 DScale=0 PME- 00: ec 10 67 81 17 00 b0 02 10 00 00 02 08 40 00 00 10: 01 d8 00 00 00 fc 8f fe 00 00 00 00 00 00 00 00 20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 45 13 30: 00 00 8c fe dc 00 00 00 00 00 00 00 0b 01 20 40 This PCI ID was added to 2.6.19. You should run 2.6.19-rc5 or backport the changes. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] IPv6/DCCP: Fix memory leak in dccp_v6_do_rcv()
Coverity found what looks like a real leak in net/dccp/ipv6.c::dccp_v6_do_rcv()
We may leave via the return inside if (sk-sk_state == DCCP_OPEN) {
but at that point we may have allocated opt_skb, but we never free it
in that path before the return.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
net/dccp/ipv6.c |2 ++
1 file changed, 2 insertions(+)
--- linux-2.6.18-git10-orig/net/dccp/ipv6.c 2006-09-28 22:40:07.0
+0200
+++ linux-2.6.18-git10/net/dccp/ipv6.c 2006-09-29 02:35:15.0 +0200
@@ -997,6 +997,8 @@ static int dccp_v6_do_rcv(struct sock *s
if (sk-sk_state == DCCP_OPEN) { /* Fast path */
if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb-len))
goto reset;
+ if (opt_skb)
+ __kfree_skb(opt_skb);
return 0;
}
PS. Please keep me on Cc:
--
Jesper Juhl [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.18-mm1 -- ieee80211: Info elem: parse failed: info_element-len + 2 left : info_element-len+2=28 left=9, id=221.
On 27/09/06, Miles Lane [EMAIL PROTECTED] wrote: On 9/26/06, Andrew Morton [EMAIL PROTECTED] wrote: [added netdev] On Tue, 26 Sep 2006 12:04:40 -0700 Miles Lane [EMAIL PROTECTED] wrote: ieee80211: Info elem: parse failed: info_element-len + 2 left : info_element-len+2=28 left=9, id=221. ieee80211: Info elem: parse failed: info_element-len + 2 left : info_element-len+2=28 left=9, id=221. ieee80211: Info elem: parse failed: info_element-len + 2 left : info_element-len+2=28 left=9, id=221. From dmesg output: ieee80211: 802.11 data/management/control stack, git-1.1.13 ieee80211: Copyright (C) 2004-2005 Intel Corporation [EMAIL PROTECTED] ieee80211_crypt: registered algorithm 'NULL' ieee80211_crypt: registered algorithm 'WEP' ieee80211_crypt: registered algorithm 'CCMP' ieee80211_crypt: registered algorithm 'TKIP' I suspect that whatever caused this is now in mainline. Are you able to test Linus's current git tree? Sorry, I don't have oodles of disk space free to hold all the git historical information (iirc, it's huge). [snip] You could just grab the latest snapshot (at the time of this writing that's 2.6.18-git8) from kernel.org - that way you wouldn't get all the historical git data. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add netpoll/netconsole support to vlan devices
On 26/09/06, David Miller [EMAIL PROTECTED] wrote: From: [EMAIL PROTECTED] Date: Mon, 25 Sep 2006 16:43:10 -0700 +#ifdef CONFIG_NET_POLL_CONTROLLER + new_dev-poll_controller = real_dev-poll_controller; +#endif + I don't see how this part can be correct. If netpoll actually calls new_dev-poll_controller it will pass new_dev in (ie. the VLAN device) yet this method is for real_dev. If the only side effect of this assignment is that netpoll recognizes the device as being usable with netpoll, that's really a nasty way to accomplish that. It was the only way I could find to accomplish that. If you know of a better way I'd appreciate a hint :) In any event, propagating this method pointer to the wrong device structure is a bug. Ok, then the patch as it stands is dead. I'll try to find another way. Thank you for your comments. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to halt or reboot due to - unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1
On 31/08/06, Jesper Juhl [EMAIL PROTECTED] wrote: On 31/08/06, Ben Greear [EMAIL PROTECTED] wrote: Jesper Juhl wrote: Hi, I've got a small problem with 2.6.18-rc5-git2. I've got a vlan setup on eth0.20, eth0 does not have an IP. When I attempt to reboot or halt the machine I get the following message from the loop in net/core/dev.c::netdev_wait_allrefs() where it waits for the ref-count to drop to zero. Unfortunately the ref-count stays at 1 forever and the server never gets any further. unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1 I googled a bit and found that people have had similar problems in the past and could work around them by shutting down the vlan interface before the 'lo' interface. I tried that and indeed, it works. Any idea how we can get this fixed? This is usually a ref-count leak somewhere. Used to be IPv6 had issues..then there were some neighbor leaks...but these were fixed as far as I know. Using IPv4 here. Can you reproduce this on older kernels? I've not actively tried, but I do have several servers running various older kernel releases with similar vlan setups and I'm not aware of any problems with those. Only this new box that I'm using for testing new kernels (currently) shows the problem, and I've only tried 2.6.8 and 2.6.18-rc5-git2 on the box so far (2.6.8 doesn't have the problem). I've just encountered the problem on a different server with an identical vlan setup. That server is running 2.6.13.4 -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to halt or reboot due to - unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1
On 01/09/06, Herbert Xu [EMAIL PROTECTED] wrote: Jesper Juhl [EMAIL PROTECTED] wrote: I've just encountered the problem on a different server with an identical vlan setup. That server is running 2.6.13.4 Do you have a simple recipe to reproduce this? Ideally it'd be a script that anyone can execute in a freshly booted system that exhibits the problem. Well, the first server I saw this on only had a base install of debian stable on it, then I replaced the kernel, configured the vlan interface in /etc/network/interfaces typed 'reboot' and it failed - and it seems to fail reliably on reboot every time. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to halt or reboot due to - unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1
On 01/09/06, Jesper Juhl [EMAIL PROTECTED] wrote: On 01/09/06, Herbert Xu [EMAIL PROTECTED] wrote: Jesper Juhl [EMAIL PROTECTED] wrote: I've just encountered the problem on a different server with an identical vlan setup. That server is running 2.6.13.4 Do you have a simple recipe to reproduce this? Ideally it'd be a script that anyone can execute in a freshly booted system that exhibits the problem. Well, the first server I saw this on only had a base install of debian stable on it, then I replaced the kernel, configured the vlan interface in /etc/network/interfaces typed 'reboot' and it failed - and it seems to fail reliably on reboot every time. Ok, I've done some more testing and it seems, unfortunately, that I can't trigger the problem reliably. I guess I was just lucky with my first few reboots. It now seems that uptime and/or amount of data that has flowed over the vlan interface impacts the probability of hitting the problem. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC][PATCH] Add netpoll/netconsole support to vlan devices
Greetings, I recently tried running netconsole via a vlan interface without luck, and decided to do something about it (see patch below). My interfaces look like this: eth0 Link encap:Ethernet HWaddr 00:14:5E:28:3C:2E UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:11763760878 errors:0 dropped:0 overruns:0 frame:0 TX packets:13800040335 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:3453951528686 (3.1 TiB) TX bytes:7086425530052 (6.4 TiB) Interrupt:169 eth0.20 Link encap:Ethernet HWaddr 00:14:5E:28:3C:2E inet addr:vvv.xxx.yyy.zzz Bcast:ggg.hhh.iii.jjj Mask:255.255.252.0 UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:11759883828 errors:0 dropped:0 overruns:0 frame:0 TX packets:13800040452 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:0 RX bytes:3194985806767 (2.9 TiB) TX bytes:6975984645540 (6.3 TiB) Trying to load netconsole fails miserably : [EMAIL PROTECTED] root]# modprobe netconsole netconsole=@/eth0,[EMAIL PROTECTED]/00:04:23:BF:D9:62 FATAL: Error inserting netconsole (/lib/modules/2.6.17.8/kernel/drivers/net/netconsole.ko): Invalid argument [EMAIL PROTECTED] root]# modprobe netconsole netconsole=@/eth0.20,[EMAIL PROTECTED]/00:04:23:BF:D9:62 FATAL: Error inserting netconsole (/lib/modules/2.6.17.8/kernel/drivers/net/netconsole.ko): Invalid argument And results in this in dmesg : netconsole: interface eth0 netconsole: remote port 10514 netconsole: remote IP aaa.bbb.ccc.ddd netconsole: remote ethernet address 00:04:23:bf:d9:62 netconsole: no IP address for eth0, aborting netconsole: local port 6665 netconsole: interface eth0.20 netconsole: remote port 10514 netconsole: remote IP aaa.bbb.ccc.ddd netconsole: remote ethernet address 00:04:23:bf:d9:62 netconsole: eth0.20 doesn't support polling, aborting. The actual interface is a e1000 and supports polling just fine. The problem is simply that netpoll won't play with eth0 since it has no IP and it won't play with eth0.20 either since it's a vlan interface and doesn't support polling even if the underlying device does. This is a problem for me since it means that several of my servers that are configured like this won't be able to use netconsole. So I set out to try and fix the problem by making the vlan device support polling if the underlying device supports polling. I have met with a resonable success in that the patch below seems to work. With this patch I can now load netconsole for eth0.20 and log messages are send just fine and can be recieved on the remote host. netconsole: local port 6665 netconsole: interface eth0.20 netconsole: remote port 10514 netconsole: remote IP aaa.bbb.ccc.ddd netconsole: remote ethernet address 00:04:23:bf:d9:62 netconsole: local IP vvv.xxx.yyy.zzz netconsole: network logging started Now, I would very much like to get this merged, but first I would appreciate some review of the patch. This part of the kernel is not one I know all too well, so I may have made a number of silly mistakes. If people would kindly take a look at the patch and point out any problems with it so I can fix it up and arrive at a version that's mergable, I'd greatly appreciate it. PS. If you reply to this on the netdev list, please keep me on Cc: since I'm only subscribed to linux-kernel. Kind regards, Jesper Juhl [EMAIL PROTECTED] Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- net/8021q/vlan.c |5 + net/8021q/vlan_dev.c | 15 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff -upr linux-2.6.18-rc5-git2-orig/net/8021q/vlan.c linux-2.6.18-rc5-git2/net/8021q/vlan.c --- linux-2.6.18-rc5-git2-orig/net/8021q/vlan.c 2006-08-31 10:07:08.0 +0200 +++ linux-2.6.18-rc5-git2/net/8021q/vlan.c 2006-08-31 16:34:33.0 +0200 @@ -11,6 +11,7 @@ * Add HW acceleration hooks - David S. Miller davem@redhat.com; * Correct all the locking - David S. Miller davem@redhat.com; * Use hash table for VLAN groups - David S. Miller davem@redhat.com + * Add NETPOLL support - Jesper Juhl * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -512,6 +513,10 @@ static struct net_device *register_vlan_ } new_dev-hard_header_parse = real_dev-hard_header_parse; +#ifdef CONFIG_NET_POLL_CONTROLLER + new_dev-poll_controller = real_dev-poll_controller; +#endif + VLAN_DEV_INFO(new_dev)-vlan_id = VLAN_ID; /* 1 through VLAN_VID_MASK */ VLAN_DEV_INFO(new_dev)-real_dev = real_dev; VLAN_DEV_INFO(new_dev)-dent = NULL; diff -upr linux-2.6.18-rc5-git2-orig/net/8021q/vlan_dev.c linux-2.6.18-rc5-git2/net/8021q/vlan_dev.c --- linux-2.6.18-rc5-git2-orig/net/8021q/vlan_dev.c 2006-06
Re: Unable to halt or reboot due to - unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1
On 31/08/06, Ben Greear [EMAIL PROTECTED] wrote: Jesper Juhl wrote: Hi, I've got a small problem with 2.6.18-rc5-git2. I've got a vlan setup on eth0.20, eth0 does not have an IP. When I attempt to reboot or halt the machine I get the following message from the loop in net/core/dev.c::netdev_wait_allrefs() where it waits for the ref-count to drop to zero. Unfortunately the ref-count stays at 1 forever and the server never gets any further. unregister_netdevice: waiting for eth0.20 to become free. Usage count = 1 I googled a bit and found that people have had similar problems in the past and could work around them by shutting down the vlan interface before the 'lo' interface. I tried that and indeed, it works. Any idea how we can get this fixed? This is usually a ref-count leak somewhere. Used to be IPv6 had issues..then there were some neighbor leaks...but these were fixed as far as I know. Using IPv4 here. Can you reproduce this on older kernels? I've not actively tried, but I do have several servers running various older kernel releases with similar vlan setups and I'm not aware of any problems with those. Only this new box that I'm using for testing new kernels (currently) shows the problem, and I've only tried 2.6.8 and 2.6.18-rc5-git2 on the box so far (2.6.8 doesn't have the problem). -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC][RESEND] remove broken URLs from net drivers' output
On 17/08/06, Markus Dahms [EMAIL PROTECTED] wrote: Remove broken URLs (www.scyld.com) from network drivers' logging output. URLs in comments and other strings are left intact. Makes good sense to me. No point in refering to URLs that are no longer operational. The best thing would be to find a replacement URL if one exist, but failing that removing the wrong ones is a good thing IMHO. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix memory leak in net/ipv4/tcp_probe.c::tcpprobe_read()
On 05/08/06, David Miller [EMAIL PROTECTED] wrote:
From: Jesper Juhl [EMAIL PROTECTED]
Date: Sat, 5 Aug 2006 01:30:49 +0200
On 31/07/06, David Miller [EMAIL PROTECTED] wrote:
From: Jesper Juhl [EMAIL PROTECTED]
Date: Sun, 30 Jul 2006 23:51:20 +0200
Looks ok to me.
I've applied James's version of the fix, thanks everyone.
Hmm, if you are refering to commit
118075b3cdc90e0815362365f3fc64d672ace0d6 -
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=118075b3cdc90e0815362365f3fc64d672ace0d6
then I think a mistake has crept in. That commit only initializes
'cnt' to 0 - I don't see how that would fix the leak - looks like you
forgot the business end of the patch...
See the commit right before that, the initialize of cnt to
zero is just to fix a compiler warning that resulted from
James's version of the fix.
Hmm, perhaps I'm going blind, but I don't see it.
The commit right before the one I linked to above is completely
unrelated : [ATALK]: Make CONFIG_DEV_APPLETALK a tristate.
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9cac2c35e26cc44978df654306bb92d7cfe7e2de
And if I download 2.6.18-rc4 the tcpprobe_read() function (still)
looks like this :
static ssize_t tcpprobe_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos)
{
int error = 0, cnt = 0;
unsigned char *tbuf;
if (!buf || len 0)
return -EINVAL;
if (len == 0)
return 0;
tbuf = vmalloc(len);
if (!tbuf)
return -ENOMEM;
error = wait_event_interruptible(tcpw.wait,
__kfifo_len(tcpw.fifo) != 0);
if (error)
return error;
cnt = kfifo_get(tcpw.fifo, tbuf, len);
error = copy_to_user(buf, tbuf, cnt);
vfree(tbuf);
return error ? error : cnt;
}
That function still contains the 'tbuf' leak.
I also couldn't find the fix in your git trees at
http://www.kernel.org/git/?p=linux/kernel/git/davem/net-2.6.19.git;a=summary
http://www.kernel.org/git/?p=linux/kernel/git/davem/net-2.6.git;a=summary
So either I'm going blind or a mistake has been made getting the fix
into mainline...
--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fix memory leak in net/ipv4/tcp_probe.c::tcpprobe_read()
There's an obvious memory leak in net/ipv4/tcp_probe.c::tcpprobe_read()
We are not freeing 'tbuf' on error.
Patch below fixes that.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
net/ipv4/tcp_probe.c |4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)
--- linux-2.6.18-rc3-orig/net/ipv4/tcp_probe.c 2006-07-30 13:21:53.0
+0200
+++ linux-2.6.18-rc3/net/ipv4/tcp_probe.c 2006-07-30 21:32:04.0
+0200
@@ -129,8 +129,10 @@ static ssize_t tcpprobe_read(struct file
error = wait_event_interruptible(tcpw.wait,
__kfifo_len(tcpw.fifo) != 0);
- if (error)
+ if (error) {
+ vfree(tbuf);
return error;
+ }
cnt = kfifo_get(tcpw.fifo, tbuf, len);
error = copy_to_user(buf, tbuf, cnt);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix memory leak in net/ipv4/tcp_probe.c::tcpprobe_read()
On 30/07/06, James Morris [EMAIL PROTECTED] wrote: On Sun, 30 Jul 2006, Stephen Hemminger wrote: On Sun, 30 Jul 2006 21:38:02 +0200 Jesper Juhl [EMAIL PROTECTED] wrote: There's an obvious memory leak in net/ipv4/tcp_probe.c::tcpprobe_read() We are not freeing 'tbuf' on error. Patch below fixes that. Signed-off-by: Jesper Juhl [EMAIL PROTECTED] Agreed, thanks for catching it. The whole kfifo interface is kind of annoying have to do an extra copy. Might be cleaner to make a single return path for cleanup: Signed-off-by: James Morris [EMAIL PROTECTED] Looks ok to me. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] networking ipv4: remove total socket usage count from /proc/net/sockstat
On 1/16/06, Andy Gospodarek [EMAIL PROTECTED] wrote: What userspace app will break because of this? On 1/16/06, Lee Revell [EMAIL PROTECTED] wrote: On Mon, 2006-01-16 at 15:04 -0500, Andy Gospodarek wrote: Printing the total number of sockets used in /proc/net/sockstat is out of place in a file that is supposed to contain information related to ipv4 sockets. Removed output for total socket usage. Um, you can't do that, it will break userspace. That's not the point. The point is you can't go around changing things exported to usersace - that has the potential to break apps. Even if no app is known to the people on this list there may still be apps out there depending on it - and we don't break userspace without *very* good reasons, and even then it's announced for several months (years sometimes) in Documentation/feature-removal.txt and elsewhere. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] networking ipv4: remove total socket usage count from /proc/net/sockstat
On 1/16/06, Andy Gospodarek [EMAIL PROTECTED] wrote: [could you *please* not top post? It's pretty annoying] Jesper, Thanks for the explanation. Your reasoning makes sense. I will I'm glad you found it useful. consider other ways to solve my current problem and post a patch that doesn't break userspace if necessary. Maybe if you described your current problem someone could suggest a solution... -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 0/3] TCP/IP Critical socket communication mechanism
On 12/14/05, Sridhar Samudrala [EMAIL PROTECTED] wrote: These set of patches provide a TCP/IP emergency communication mechanism that could be used to guarantee high priority communications over a critical socket to succeed even under very low memory conditions that last for a couple of minutes. It uses the critical page pool facility provided by Matt's patches that he posted recently on lkml. http://lkml.org/lkml/2005/12/14/34/index.html This mechanism provides a new socket option SO_CRITICAL that can be used to mark a socket as critical. A critical connection used for emergency So now everyone writing commercial apps for Linux are going to set SO_CRITICAL on sockets in their apps so their apps can survive better under pressure than the competitors aps and clueless programmers all over are going to think cool, with this I can make my app more important than everyone elses, I'm going to use this. When everyone and his dog starts to set this, what's the point? communications has to be established and marked as critical before we enter the emergency condition. It uses the __GFP_CRITICAL flag introduced in the critical page pool patches to indicate an allocation request as critical and should be satisfied from the critical page pool if required. In the send path, this flag is passed with all allocation requests that are made for a critical socket. But in the receive path we do not know if a packet is critical or not until we receive it and find the socket that it is destined to. So we treat all the allocation requests in the receive path as critical. The critical page pool patches also introduces a global flag 'system_in_emergency' that is used to indicate an emergency situation(could be a low memory condition). When this flag is set any incoming packets that belong to non-critical sockets are dropped as soon as possible in the receive path. Hmm, so if I fire up an app that has SO_CRITICAL set on a socket and can then somehow put a lot of memory pressure on the machine I can cause traffic on other sockets to be dropped.. hmmm.. sounds like something to play with to create new and interresting DoS attacks... This is necessary to prevent incoming non-critical packets to consume memory from critical page pool. I would appreciate any feedback or comments on this approach. To be a little serious, it sounds like something that could be used to cause trouble and something that will lose its usefulness once enough people start using it (for valid or invalid reasons), so what's the point... -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] net: Remove unneeded kmalloc() return value casts
Get rid of needless casting of kmalloc() return value in net/
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
net/bluetooth/hci_conn.c |2 +-
net/sunrpc/svc.c |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
--- linux-2.6.15-rc5-git1-orig/net/bluetooth/hci_conn.c 2005-10-28
02:02:08.0 +0200
+++ linux-2.6.15-rc5-git1/net/bluetooth/hci_conn.c 2005-12-11
19:50:32.0 +0100
@@ -403,7 +403,7 @@ int hci_get_conn_list(void __user *arg)
size = sizeof(req) + req.conn_num * sizeof(*ci);
- if (!(cl = (void *) kmalloc(size, GFP_KERNEL)))
+ if (!(cl = kmalloc(size, GFP_KERNEL)))
return -ENOMEM;
if (!(hdev = hci_dev_get(req.dev_id))) {
--- linux-2.6.15-rc5-git1-orig/net/sunrpc/svc.c 2005-12-04 18:49:00.0
+0100
+++ linux-2.6.15-rc5-git1/net/sunrpc/svc.c 2005-12-11 19:54:05.0
+0100
@@ -167,8 +167,8 @@ svc_create_thread(svc_thread_fn func, st
memset(rqstp, 0, sizeof(*rqstp));
init_waitqueue_head(rqstp-rq_wait);
- if (!(rqstp-rq_argp = (u32 *) kmalloc(serv-sv_xdrsize, GFP_KERNEL))
-|| !(rqstp-rq_resp = (u32 *) kmalloc(serv-sv_xdrsize, GFP_KERNEL))
+ if (!(rqstp-rq_argp = kmalloc(serv-sv_xdrsize, GFP_KERNEL))
+|| !(rqstp-rq_resp = kmalloc(serv-sv_xdrsize, GFP_KERNEL))
|| !svc_init_buffer(rqstp, serv-sv_bufsz))
goto out_thread;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] ip / ifconfig redesign
On 12/2/05, linux-os (Dick Johnson) [EMAIL PROTECTED] wrote: On Fri, 2 Dec 2005, Al Boldi wrote: The current ip / ifconfig configuration is arcane and inflexible. The reason being, that they are based on design principles inherited from the last century. In a GNU/OpenSource environment, OpenMinds should not inhibit themselves achieving new design-goals to enable a flexible non-redundant configuration. Specifically, '# ip addr ' exhibits this issue clearly, by requiring to associate the address to a link instead of the other way around. Consider this new approach for better address management: 1. Allow the definition of an address pool 2. Relate links to addresses 3. Implement to make things backward-compatible. The obvious benefit here, would be the transparent ability for apps to bind to addresses, regardless of the link existence. A link needs to exist for it to have an address. I'm only guessing since I'm not entirely sure what Mr. Boldi means, but my guess is that he's proposing that an app can bind to an IP address without that address being assigned to any currently available interface and then later if that IP does get assigned to an interface the app will start recieving traffic then. Also possibly allowing the address to be removed from one interface and then later assigned to another one without apps noticing. I don't know /if/ that is what was meant, but that's how I read it. Another benefit includes the ability to scale the link level transparently, regardless of the application bind state. That doesn't make any sense. Multiple applications can bind to the same address. Then can't bind to the same port because they won't get all their data. And there may be many other benefits... (i.e. 100% OSI compliance) What does Open Source Initiative have to do with this at all??? You are just spewing stuff out. I'm believe Mr. Boldi is refering to OSI as in Open Systems Interconnection (http://en.wikipedia.org/wiki/OSI_model), not as in Open Source Initiative. -- Al Also, how does this involve the kernel? The interface to the kernel for hardware configuration is via ioctl(). For logic, sockets. If the user-level tools suck, then they should be fixed. It really doesn't have anything to do with the kernel. Cheers, Dick Johnson [snip] Thank you. -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] wrong firmware location in IPW2100 Kconfig entry (Was: IPW2100 Kconfig)
On Tuesday 06 September 2005 20:32, Alejandro Bonilla wrote: Hi, I checked the IPW2100 in the current git from linux-2.6 and the menuconfig help (Kconfig) says you need to put the firmware in /etc/firmware, it should be /lib/firmware. Who should I send the patch to? Or can someone simply change that? Firmware should go into /lib/firmware, not /etc/firmware. Found by Alejandro Bonilla. Signed-off-by: Jesper Juhl [EMAIL PROTECTED] --- drivers/net/wireless/Kconfig |2 +- 1 files changed, 1 insertion(+), 1 deletion(-) --- linux-2.6.13-mm1-orig/drivers/net/wireless/Kconfig 2005-09-02 23:59:51.0 +0200 +++ linux-2.6.13-mm1/drivers/net/wireless/Kconfig 2005-09-06 20:39:45.0 +0200 @@ -152,7 +152,7 @@ In order to use this driver, you will need a firmware image for it. You can obtain the firmware from http://ipw2100.sf.net/. Once you have the firmware image, you - will need to place it in /etc/firmware. + will need to place it in /lib/firmware. You will also very likely need the Wireless Tools in order to configure your card: - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: igmp problem
On 9/5/05, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote: Hi! I wrote a few multicast tools, which use these multicast groups: 225.109.99.112 ff02::6d63:7030 224.110.99.112 ff02::6e63:7030 I have a Cisco in the middle, and both boxes are in different VLANs. The Cisco is sending out igmp queries. The kernel never answers, even after subscribing to these multicast groups. The kernel version is 2.4.26. What could be the problem here? I found no netfilter rules, and the kernel has multicast support (at least several igmp related sysctls exist). I'm unfortunately not able to help you with your specific problem, but a few words of advice: you really ought to start by reproducing the problem with a recent kernel - like 2.4.31 or 2.6.13. 2.4.26 is really ancient and noone really cares about it any more. You should probably also talk to the netdev people (CC'ed). -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible BUG in IPv4 TCP window handling, all recent 2.4.x/2.6.x kernels
On 9/2/05, Ion Badulescu [EMAIL PROTECTED] wrote: Hi David, On Thu, 1 Sep 2005, David S. Miller wrote: Thanks for the empty posting. Please provide the content you intended to post, and furthermore please post it to the network developer mailing list, netdev@vger.kernel.org First of all, thanks for the reply (even to an empty posting :). The posting wasn't actually empty, it was probably too long (94K according Two solutions commonly applied to that problem : - put the big file(s) online somewhere and include an URL in the email - compress the file(s) and attach the compressed files to the email -- Jesper Juhl [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto_free_tfm callers do not need to check for NULL
Since the patch to add a NULL short-circuit to crypto_free_tfm() went in,
there's no longer any need for callers of that function to check for NULL.
This patch removes the redundant NULL checks and also a few similar checks
for NULL before calls to kfree() that I ran into while doing the
crypto_free_tfm bits.
I've posted similar patches in the past, but was asked to first until the
short-circuit patch moved from -mm to mainline - and since it is now
firmly there in 2.6.13 I assume there's no problem there anymore.
I was also asked previously to make the patch against mainline and not -mm,
so this patch is against 2.6.13.
Feedback, ACK, NACK, etc welcome.
Sorry about the large Cc list, but I wanted to include everyone involved
with the code I change.
Please keep me on Cc.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
./drivers/net/wireless/airo.c |3 +--
./fs/nfsd/nfs4recover.c |3 +--
./net/ipv4/ah4.c| 18 ++
./net/ipv4/esp4.c | 24
./net/ipv4/ipcomp.c |3 +--
./net/ipv6/addrconf.c |6 ++
./net/ipv6/ah6.c| 18 ++
./net/ipv6/esp6.c | 24
./net/ipv6/ipcomp6.c|3 +--
./net/sctp/socket.c |3 +--
./net/sunrpc/auth_gss/gss_krb5_crypto.c |3 +--
./net/sunrpc/auth_gss/gss_krb5_mech.c |9 +++--
./net/sunrpc/auth_gss/gss_spkm3_mech.c | 12
13 files changed, 43 insertions(+), 86 deletions(-)
--- linux-2.6.13-orig/./drivers/net/wireless/airo.c 2005-08-29
01:41:01.0 +0200
+++ linux-2.6.13/./drivers/net/wireless/airo.c 2005-08-30 18:08:15.0
+0200
@@ -2403,8 +2403,7 @@ void stop_airo_card( struct net_device *
}
}
#ifdef MICSUPPORT
- if (ai-tfm)
- crypto_free_tfm(ai-tfm);
+ crypto_free_tfm(ai-tfm);
#endif
del_airo_dev( dev );
free_netdev( dev );
--- linux-2.6.13-orig/./fs/nfsd/nfs4recover.c 2005-08-29 01:41:01.0
+0200
+++ linux-2.6.13/./fs/nfsd/nfs4recover.c2005-08-30 18:08:25.0
+0200
@@ -114,8 +114,7 @@ nfs4_make_rec_clidname(char *dname, stru
kfree(cksum.data);
status = nfs_ok;
out:
- if (tfm)
- crypto_free_tfm(tfm);
+ crypto_free_tfm(tfm);
return status;
}
--- linux-2.6.13-orig/./net/ipv4/ah4.c 2005-08-29 01:41:01.0 +0200
+++ linux-2.6.13/./net/ipv4/ah4.c 2005-08-30 18:10:10.0 +0200
@@ -263,10 +263,8 @@ static int ah_init_state(struct xfrm_sta
error:
if (ahp) {
- if (ahp-work_icv)
- kfree(ahp-work_icv);
- if (ahp-tfm)
- crypto_free_tfm(ahp-tfm);
+ kfree(ahp-work_icv);
+ crypto_free_tfm(ahp-tfm);
kfree(ahp);
}
return -EINVAL;
@@ -279,14 +277,10 @@ static void ah_destroy(struct xfrm_state
if (!ahp)
return;
- if (ahp-work_icv) {
- kfree(ahp-work_icv);
- ahp-work_icv = NULL;
- }
- if (ahp-tfm) {
- crypto_free_tfm(ahp-tfm);
- ahp-tfm = NULL;
- }
+ kfree(ahp-work_icv);
+ ahp-work_icv = NULL;
+ crypto_free_tfm(ahp-tfm);
+ ahp-tfm = NULL;
kfree(ahp);
}
--- linux-2.6.13-orig/./net/ipv4/esp4.c 2005-08-29 01:41:01.0 +0200
+++ linux-2.6.13/./net/ipv4/esp4.c 2005-08-30 18:10:49.0 +0200
@@ -343,22 +343,14 @@ static void esp_destroy(struct xfrm_stat
if (!esp)
return;
- if (esp-conf.tfm) {
- crypto_free_tfm(esp-conf.tfm);
- esp-conf.tfm = NULL;
- }
- if (esp-conf.ivec) {
- kfree(esp-conf.ivec);
- esp-conf.ivec = NULL;
- }
- if (esp-auth.tfm) {
- crypto_free_tfm(esp-auth.tfm);
- esp-auth.tfm = NULL;
- }
- if (esp-auth.work_icv) {
- kfree(esp-auth.work_icv);
- esp-auth.work_icv = NULL;
- }
+ crypto_free_tfm(esp-conf.tfm);
+ esp-conf.tfm = NULL;
+ kfree(esp-conf.ivec);
+ esp-conf.ivec = NULL;
+ crypto_free_tfm(esp-auth.tfm);
+ esp-auth.tfm = NULL;
+ kfree(esp-auth.work_icv);
+ esp-auth.work_icv = NULL;
kfree(esp);
}
--- linux-2.6.13-orig/./net/ipv4/ipcomp.c 2005-08-29 01:41:01.0
+0200
+++ linux-2.6.13/./net/ipv4/ipcomp.c2005-08-30 18:11:14.0 +0200
@@ -345,8 +345,7 @@ static void ipcomp_free_tfms(struct cryp
for_each_cpu(cpu) {
struct crypto_tfm *tfm = *per_cpu_ptr(tfms, cpu);
- if (tfm)
- crypto_free_tfm(tfm);
+ crypto_free_tfm(tfm
