Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

>Amit Choudhary <[EMAIL PROTECTED]> wrote:
>> --- Christoph Hellwig <[EMAIL PROTECTED]> wrote:
>>> On Sun, Jan 07, 2007 at 12:46:50AM -0800, Amit Choudhary wrote:

>> Any strong reason why not? x has some value that does not make sense and can
>> create only problems. And as I explained, it can result in longer code too.
>> So, why keep this value around. Why not re-initialize it to NULL.

>1) Because some magic value like 0x23 would be better.
>2) Because it hides bugs like double frees or dangeling references while
>creating a race condition. In the end, you'll get more hard-to-find bugs
>in exchange for papering over some easy-to-spot bugs.

Sorry for spamming the list again but some people may find this useful:

[Dangling pointers are security vulnerability]

http://it.slashdot.org/it/07/07/23/1624203.shtml

Excerpt:
***
Dangling pointers are quite common, but security experts and developers have 
said for years that
there is no practical way to exploit them, so they've been considered 
quality-assurance problems
and not security flaws.

But now that has changed.

"The problem before was, you had to override the exact location that the 
pointer was pointing to.
It was considered impossible. But we discovered a way to do this with generic 
dangling pointers
and run our own shell code."
***

-Amit



   

Take the Internet to Go: Yahoo!Go puts the Internet in your pocket: mail, news, 
photos & more. 
http://mobile.yahoo.com/go?refer=1GNXIC
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

Amit Choudhary [EMAIL PROTECTED] wrote:
 --- Christoph Hellwig [EMAIL PROTECTED] wrote:
 On Sun, Jan 07, 2007 at 12:46:50AM -0800, Amit Choudhary wrote:

 Any strong reason why not? x has some value that does not make sense and can
 create only problems. And as I explained, it can result in longer code too.
 So, why keep this value around. Why not re-initialize it to NULL.

1) Because some magic value like 0x23 would be better.
2) Because it hides bugs like double frees or dangeling references while
creating a race condition. In the end, you'll get more hard-to-find bugs
in exchange for papering over some easy-to-spot bugs.

Sorry for spamming the list again but some people may find this useful:

[Dangling pointers are security vulnerability]

http://it.slashdot.org/it/07/07/23/1624203.shtml

Excerpt:
***
Dangling pointers are quite common, but security experts and developers have 
said for years that
there is no practical way to exploit them, so they've been considered 
quality-assurance problems
and not security flaws.

But now that has changed.

The problem before was, you had to override the exact location that the 
pointer was pointing to.
It was considered impossible. But we discovered a way to do this with generic 
dangling pointers
and run our own shell code.
***

-Amit



   

Take the Internet to Go: Yahoo!Go puts the Internet in your pocket: mail, news, 
photos  more. 
http://mobile.yahoo.com/go?refer=1GNXIC
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/videocodec.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
videocodec_build_table(), in file drivers/media/video/videocodec.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/videocodec.c b/drivers/media/video/videocodec.c
index 290e641..f2bbd7a 100644
--- a/drivers/media/video/videocodec.c
+++ b/drivers/media/video/videocodec.c
@@ -348,6 +348,9 @@ #define LINESIZE 100
kfree(videocodec_buf);
videocodec_buf = kmalloc(size, GFP_KERNEL);
 
+   if (!videocodec_buf)
+   return 0;
+
i = 0;
i += scnprintf(videocodec_buf + i, size - 1,
  "lave or attached aster name  type flagsmagic   
 ");
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/se401.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
se401_start_stream(), in file drivers/media/video/se401.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/se401.c b/drivers/media/video/se401.c
index 7aeec57..006c818 100644
--- a/drivers/media/video/se401.c
+++ b/drivers/media/video/se401.c
@@ -450,6 +450,13 @@ static int se401_start_stream(struct usb
}
for (i=0; isbuf[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401->sbuf[i].data) {
+   for(i = i - 1; i >= 0; i--) {
+   kfree(se401->sbuf[i].data);
+   se401->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+   }
}
 
se401->bayeroffset=0;
@@ -458,13 +465,26 @@ static int se401_start_stream(struct usb
se401->scratch_overflow=0;
for (i=0; iscratch[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401->scratch[i].data) {
+   for(i = i - 1; i >= 0; i--) {
+   kfree(se401->scratch[i].data);
+   se401->scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
+   }
se401->scratch[i].state=BUFFER_UNUSED;
}
 
for (i=0; i= 0; i--) {
+   usb_kill_urb(se401->urb[i]);
+   usb_free_urb(se401->urb[i]);
+   se401->urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
usb_fill_bulk_urb(urb, se401->dev,
usb_rcvbulkpipe(se401->dev, SE401_VIDEO_ENDPOINT),
@@ -482,6 +502,18 @@ static int se401_start_stream(struct usb
se401->framecount=0;
 
return 0;
+
+ nomem_scratch:
+   for (i=0; iscratch[i].data);
+   se401->scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; isbuf[i].data);
+   se401->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int se401_stop_stream(struct usb_se401 *se401)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..f35c664 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,11 @@ static int stv680_start_stream (struct u
stv680->sbuf[i].data = kmalloc (stv680->rawbufsize, GFP_KERNEL);
if (stv680->sbuf[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw data buffer 
%i", i);
-   return -1;
+   for (i = i - 1; i >= 0; i--) {
+   kfree(stv680->sbuf[i].data);
+   stv680->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
}
}
 
@@ -698,15 +702,25 @@ static int stv680_start_stream (struct u
stv680->scratch[i].data = kmalloc (stv680->rawbufsize, 
GFP_KERNEL);
if (stv680->scratch[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw scratch 
buffer %i", i);
-   return -1;
+   for (i = i - 1; i >= 0; i--) {
+   kfree(stv680->scratch[i].data);
+   stv680->scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
}
stv680->scratch[i].state = BUFFER_UNUSED;
}
 
for (i = 0; i < STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
-   if (!urb)
-   return -ENOMEM;
+   if (!urb) {
+   for (i = i - 1; i >= 0; i--) {
+   usb_kill_urb(stv680->urb[i]);
+   usb_free_urb(stv680->urb[i]);
+   stv680->urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
/* sbuf is urb->transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680->udev,
@@ -721,6 +735,18 @@ static int stv680_start_stream (struct u
 
stv680->framecount = 0;
return 0;
+
+ nomem_scratch:
+   for (i=0; iscratch[i].data);
+   stv680->scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; isbuf[i].data);
+   stv680->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/se401.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
se401_start_stream(), in file drivers/media/video/se401.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/se401.c b/drivers/media/video/se401.c
index 7aeec57..006c818 100644
--- a/drivers/media/video/se401.c
+++ b/drivers/media/video/se401.c
@@ -450,6 +450,13 @@ static int se401_start_stream(struct usb
}
for (i=0; iSE401_NUMSBUF; i++) {
se401-sbuf[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401-sbuf[i].data) {
+   for(i = i - 1; i = 0; i--) {
+   kfree(se401-sbuf[i].data);
+   se401-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+   }
}
 
se401-bayeroffset=0;
@@ -458,13 +465,26 @@ static int se401_start_stream(struct usb
se401-scratch_overflow=0;
for (i=0; iSE401_NUMSCRATCH; i++) {
se401-scratch[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401-scratch[i].data) {
+   for(i = i - 1; i = 0; i--) {
+   kfree(se401-scratch[i].data);
+   se401-scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
+   }
se401-scratch[i].state=BUFFER_UNUSED;
}
 
for (i=0; iSE401_NUMSBUF; i++) {
urb=usb_alloc_urb(0, GFP_KERNEL);
-   if(!urb)
-   return -ENOMEM;
+   if(!urb) {
+   for(i = i - 1; i = 0; i--) {
+   usb_kill_urb(se401-urb[i]);
+   usb_free_urb(se401-urb[i]);
+   se401-urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
usb_fill_bulk_urb(urb, se401-dev,
usb_rcvbulkpipe(se401-dev, SE401_VIDEO_ENDPOINT),
@@ -482,6 +502,18 @@ static int se401_start_stream(struct usb
se401-framecount=0;
 
return 0;
+
+ nomem_scratch:
+   for (i=0; iSE401_NUMSCRATCH; i++) {
+   kfree(se401-scratch[i].data);
+   se401-scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; iSE401_NUMSBUF; i++) {
+   kfree(se401-sbuf[i].data);
+   se401-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int se401_stop_stream(struct usb_se401 *se401)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/videocodec.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
videocodec_build_table(), in file drivers/media/video/videocodec.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/videocodec.c b/drivers/media/video/videocodec.c
index 290e641..f2bbd7a 100644
--- a/drivers/media/video/videocodec.c
+++ b/drivers/media/video/videocodec.c
@@ -348,6 +348,9 @@ #define LINESIZE 100
kfree(videocodec_buf);
videocodec_buf = kmalloc(size, GFP_KERNEL);
 
+   if (!videocodec_buf)
+   return 0;
+
i = 0;
i += scnprintf(videocodec_buf + i, size - 1,
  Slave or attached Master name  type flagsmagic   
 );
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..f35c664 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,11 @@ static int stv680_start_stream (struct u
stv680-sbuf[i].data = kmalloc (stv680-rawbufsize, GFP_KERNEL);
if (stv680-sbuf[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw data buffer 
%i, i);
-   return -1;
+   for (i = i - 1; i = 0; i--) {
+   kfree(stv680-sbuf[i].data);
+   stv680-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
}
}
 
@@ -698,15 +702,25 @@ static int stv680_start_stream (struct u
stv680-scratch[i].data = kmalloc (stv680-rawbufsize, 
GFP_KERNEL);
if (stv680-scratch[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw scratch 
buffer %i, i);
-   return -1;
+   for (i = i - 1; i = 0; i--) {
+   kfree(stv680-scratch[i].data);
+   stv680-scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
}
stv680-scratch[i].state = BUFFER_UNUSED;
}
 
for (i = 0; i  STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
-   if (!urb)
-   return -ENOMEM;
+   if (!urb) {
+   for (i = i - 1; i = 0; i--) {
+   usb_kill_urb(stv680-urb[i]);
+   usb_free_urb(stv680-urb[i]);
+   stv680-urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
/* sbuf is urb-transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680-udev,
@@ -721,6 +735,18 @@ static int stv680_start_stream (struct u
 
stv680-framecount = 0;
return 0;
+
+ nomem_scratch:
+   for (i=0; iSTV680_NUMSCRATCH; i++) {
+   kfree(stv680-scratch[i].data);
+   stv680-scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; iSTV680_NUMSBUF; i++) {
+   kfree(stv680-sbuf[i].data);
+   stv680-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

getting memory for a limited time.

[Amit Choudhary]

Hi,

Is there any debug option where I get some memory (from kmalloc() family) for 
limited time only.
Once the time slice expires, that memory is zeroed out so that if I use it 
again after the time
slice expired, I should see a crash.

-Amit


 

Finding fabulous fares is fun.  
Let Yahoo! FareChase search your favorite travel sites to find flight and hotel 
bargains.
http://farechase.yahoo.com/promo-generic-14795097
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

getting memory for a limited time.

[Amit Choudhary]

Hi,

Is there any debug option where I get some memory (from kmalloc() family) for 
limited time only.
Once the time slice expires, that memory is zeroed out so that if I use it 
again after the time
slice expired, I should see a crash.

-Amit


 

Finding fabulous fares is fun.  
Let Yahoo! FareChase search your favorite travel sites to find flight and hotel 
bargains.
http://farechase.yahoo.com/promo-generic-14795097
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/se401.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
se401_start_stream(), in file drivers/media/video/se401.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/se401.c b/drivers/media/video/se401.c
index 7aeec57..006c818 100644
--- a/drivers/media/video/se401.c
+++ b/drivers/media/video/se401.c
@@ -450,6 +450,13 @@ static int se401_start_stream(struct usb
}
for (i=0; isbuf[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401->sbuf[i].data) {
+   for(i = i - 1; i >= 0; i--) {
+   kfree(se401->sbuf[i].data);
+   se401->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+   }
}
 
se401->bayeroffset=0;
@@ -458,13 +465,26 @@ static int se401_start_stream(struct usb
se401->scratch_overflow=0;
for (i=0; iscratch[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401->scratch[i].data) {
+   for(i = i - 1; i >= 0; i--) {
+   kfree(se401->scratch[i].data);
+   se401->scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
+   }
se401->scratch[i].state=BUFFER_UNUSED;
}
 
for (i=0; i= 0; i--) {
+   usb_kill_urb(se401->urb[i]);
+   usb_free_urb(se401->urb[i]);
+   se401->urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
usb_fill_bulk_urb(urb, se401->dev,
usb_rcvbulkpipe(se401->dev, SE401_VIDEO_ENDPOINT),
@@ -482,6 +502,18 @@ static int se401_start_stream(struct usb
se401->framecount=0;
 
return 0;
+
+ nomem_scratch:
+   for (i=0; iscratch[i].data);
+   se401->scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; isbuf[i].data);
+   se401->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int se401_stop_stream(struct usb_se401 *se401)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/vt.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function con_init(), in 
file drivers/char/vt.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index 87587b4..6aa08cb 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -2640,6 +2640,15 @@ static int __init con_init(void)
 */
for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
vc_cons[currcons].d = vc = alloc_bootmem(sizeof(struct 
vc_data));
+   if (!vc_cons[currcons].d) {
+   for (--currcons; currcons >= 0; currcons--) {
+   kfree(vc_cons[currcons].d);
+   vc_cons[currcons].d = NULL;
+   }
+   release_console_sem();
+   return -ENOMEM;
+   }
+
visual_init(vc, currcons, 1);
vc->vc_screenbuf = (unsigned short 
*)alloc_bootmem(vc->vc_screenbuf_size);
vc->vc_kmalloced = 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs/jffs2/scan.c: Fix error-path leak

[Amit Choudhary]

Description: Fix error-path leak in function jffs2_scan_medium(), in file 
fs/jffs2/scan.c

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index e241346..cd9ed6e 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -130,6 +130,8 @@ #endif
if (jffs2_sum_active()) {
s = kmalloc(sizeof(struct jffs2_summary), GFP_KERNEL);
if (!s) {
+   free(flashbuf);
+   flashbuf = NULL;
JFFS2_WARNING("Can't allocate memory for summary\n");
return -ENOMEM;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/agp/sgi-agp.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function agp_sgi_init(), in 
file drivers/char/agp/sgi-agp.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c
index d73be4c..5897e6c 100644
--- a/drivers/char/agp/sgi-agp.c
+++ b/drivers/char/agp/sgi-agp.c
@@ -285,6 +285,8 @@ static int __devinit agp_sgi_init(void)
(struct agp_bridge_data **)kmalloc(tioca_gart_found *
   sizeof(struct agp_bridge_data *),
   GFP_KERNEL);
+   if (!sgi_tioca_agp_bridges)
+   return -ENOMEM;
 
j = 0;
list_for_each_entry(info, _list, ca_list) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] net/wanrouter/wanmain.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function dbg_kmalloc(), in 
file net/wanrouter/wanmain.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/net/wanrouter/wanmain.c b/net/wanrouter/wanmain.c
index 316211d..263450c 100644
--- a/net/wanrouter/wanmain.c
+++ b/net/wanrouter/wanmain.c
@@ -67,6 +67,8 @@ static void * dbg_kmalloc(unsigned int s
int i = 0;
void * v = kmalloc(size+sizeof(unsigned int)+2*KMEM_SAFETYZONE*8,prio);
char * c1 = v;
+   if (!v)
+   return NULL;
c1 += sizeof(unsigned int);
*((unsigned int *)v) = size;
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/atm/fore200e.c: change in error message.

[Amit Choudhary]

Description: Change in error message in function fore200e_kmalloc(), in file 
drivers/atm/fore200e.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 3a7b21f..1c7ea02 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -178,7 +178,7 @@ fore200e_kmalloc(int size, gfp_t flags)
 void *chunk = kzalloc(size, flags);
 
 if (!chunk)
-   printk(FORE200E "kmalloc() failed, requested size = %d, flags = 
0x%x\n",size, flags);
+   printk(FORE200E "kzalloc() failed, requested size = %d, flags = 
0x%x\n",size, flags);
 
 return chunk;
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/synclink.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
mgsl_alloc_intermediate_txbuffer_memory(), in file drivers/char/synclink.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c
index 06784ad..24f99bc 100644
--- a/drivers/char/synclink.c
+++ b/drivers/char/synclink.c
@@ -4012,8 +4012,13 @@ static int mgsl_alloc_intermediate_txbuf
for ( i=0; inum_tx_holding_buffers; ++i) {
info->tx_holding_buffers[i].buffer =
kmalloc(info->max_frame_size, GFP_KERNEL);
-   if ( info->tx_holding_buffers[i].buffer == NULL )
+   if (info->tx_holding_buffers[i].buffer == NULL) {
+   for (--i; i >= 0; i--) {
+   kfree(info->tx_holding_buffers[i].buffer);
+   info->tx_holding_buffers[i].buffer = NULL;
+   }
return -ENOMEM;
+   }
}
 
return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] sound/oss/i810_audio.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function i810_open(), in 
file sound/oss/i810_audio.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/sound/oss/i810_audio.c b/sound/oss/i810_audio.c
index 240cc79..a415967 100644
--- a/sound/oss/i810_audio.c
+++ b/sound/oss/i810_audio.c
@@ -2580,8 +2580,13 @@ static int i810_open(struct inode *inode
if (card->states[i] == NULL) {
state = card->states[i] = (struct i810_state *)
kmalloc(sizeof(struct i810_state), 
GFP_KERNEL);
-   if (state == NULL)
+   if (state == NULL) {
+   for (--i; i >= 0; i--) {
+   kfree(card->states[i]);
+   card->states[i] = NULL;
+   }
return -ENOMEM;
+   }
memset(state, 0, sizeof(struct i810_state));
dmabuf = >dmabuf;
goto found_virt;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] net/ipv4/multipath_wrandom.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
wrandom_set_nhinfo(), in file net/ipv4/multipath_wrandom.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/net/ipv4/multipath_wrandom.c b/net/ipv4/multipath_wrandom.c
index 92b0482..bcdb1f1 100644
--- a/net/ipv4/multipath_wrandom.c
+++ b/net/ipv4/multipath_wrandom.c
@@ -242,6 +242,9 @@ static void wrandom_set_nhinfo(__be32 ne
target_route = (struct multipath_route *)
kmalloc(size_rt, GFP_ATOMIC);
 
+   if (!target_route)
+   goto error;
+
target_route->gw = nh->nh_gw;
target_route->oif = nh->nh_oif;
memset(_route->rcu, 0, sizeof(struct rcu_head));
@@ -263,6 +266,9 @@ static void wrandom_set_nhinfo(__be32 ne
target_dest = (struct multipath_dest*)
kmalloc(size_dst, GFP_ATOMIC);
 
+   if (!target_dest)
+   goto error;
+
target_dest->nh_info = nh;
target_dest->network = network;
target_dest->netmask = netmask;
@@ -275,6 +281,7 @@ static void wrandom_set_nhinfo(__be32 ne
 * we are finished
 */
 
+ error:
spin_unlock_bh([state_idx].lock);
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..f35c664 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,11 @@ static int stv680_start_stream (struct u
stv680->sbuf[i].data = kmalloc (stv680->rawbufsize, GFP_KERNEL);
if (stv680->sbuf[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw data buffer 
%i", i);
-   return -1;
+   for (i = i - 1; i >= 0; i--) {
+   kfree(stv680->sbuf[i].data);
+   stv680->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
}
}
 
@@ -698,15 +702,25 @@ static int stv680_start_stream (struct u
stv680->scratch[i].data = kmalloc (stv680->rawbufsize, 
GFP_KERNEL);
if (stv680->scratch[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw scratch 
buffer %i", i);
-   return -1;
+   for (i = i - 1; i >= 0; i--) {
+   kfree(stv680->scratch[i].data);
+   stv680->scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
}
stv680->scratch[i].state = BUFFER_UNUSED;
}
 
for (i = 0; i < STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
-   if (!urb)
-   return -ENOMEM;
+   if (!urb) {
+   for (i = i - 1; i >= 0; i--) {
+   usb_kill_urb(stv680->urb[i]);
+   usb_free_urb(stv680->urb[i]);
+   stv680->urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
/* sbuf is urb->transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680->udev,
@@ -721,6 +735,18 @@ static int stv680_start_stream (struct u
 
stv680->framecount = 0;
return 0;
+
+ nomem_scratch:
+   for (i=0; iscratch[i].data);
+   stv680->scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; isbuf[i].data);
+   stv680->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mm/slab.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function setup_cpu_cache(), 
in file mm/slab.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/mm/slab.c b/mm/slab.c
index 84c631f..613ae61 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2021,6 +2021,7 @@ static int setup_cpu_cache(struct kmem_c
} else {
cachep->array[smp_processor_id()] =
kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+   BUG_ON(!cachep->array[smp_processor_id()]);
 
if (g_cpucache_up == PARTIAL_AC) {
set_up_list3s(cachep, SIZE_L3);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs/cifs/readdir.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function cifs_readdir(), in 
file fs/cifs/readdir.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index b5b0a2a..2d43b2a 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -1063,6 +1063,11 @@ int cifs_readdir(struct file *file, void
such multibyte target UTF-8 characters. cifs_unicode.c,
which actually does the conversion, has the same limit */
tmp_buf = kmalloc((2 * NAME_MAX) + 4, GFP_KERNEL);
+   if (!tmp_buf) {
+   cERROR(1, ("No memory!"));
+   rc = -ENOMEM;
+   goto rddir2_exit;
+   }
for(i=0;(ihttp://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/usb/serial/mos7840.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function mos7840_get_reg(), 
in file drivers/usb/serial/mos7840.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 021be39..91d474b 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
int ret = 0;
buffer = (__u8 *) mcs->ctrl_buf;
 
+   /* The memory for ctrl_buf is allocated in
+* mos7840_startup(), but it is not checked if
+* kmalloc failed. So, mcs->ctrl_buf might be NULL.
+* So, it should be checked here.
+*/
+   if (!buffer)
+   return -ENOMEM;
+
 //  dr=(struct usb_ctrlrequest *)(buffer);
dr = (void *)(buffer + 2);
dr->bRequestType = MCS_RD_RTYPE;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..a1ec3ac 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,7 @@ static int stv680_start_stream (struct u
stv680->sbuf[i].data = kmalloc (stv680->rawbufsize, GFP_KERNEL);
if (stv680->sbuf[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw data buffer 
%i", i);
-   return -1;
+   goto nomem_err;
}
}
 
@@ -698,7 +698,7 @@ static int stv680_start_stream (struct u
stv680->scratch[i].data = kmalloc (stv680->rawbufsize, 
GFP_KERNEL);
if (stv680->scratch[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw scratch 
buffer %i", i);
-   return -1;
+   goto nomem_err;
}
stv680->scratch[i].state = BUFFER_UNUSED;
}
@@ -706,7 +706,7 @@ static int stv680_start_stream (struct u
for (i = 0; i < STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
if (!urb)
-   return -ENOMEM;
+   goto nomem_err;
 
/* sbuf is urb->transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680->udev,
@@ -721,6 +721,21 @@ static int stv680_start_stream (struct u
 
stv680->framecount = 0;
return 0;
+
+ nomem_err:
+   for (i = 0; i < STV680_NUMSCRATCH; i++) {
+   kfree(stv680->scratch[i].data);
+   stv680->scratch[i].data = NULL;
+   }
+   for (i = 0; i < STV680_NUMSBUF; i++) {
+   usb_kill_urb(stv680->urb[i]);
+   usb_free_urb(stv680->urb[i]);
+   stv680->urb[i] = NULL;
+   kfree(stv680->sbuf[i].data);
+   stv680->sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scsi: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function ch_readconfig(), 
in file drivers/scsi/ch.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index f6caa43..fcd635b 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -324,7 +324,7 @@ ch_readconfig(scsi_changer *ch)
if (!buffer)
return -ENOMEM;
memset(buffer,0,512);
-   
+
memset(cmd,0,sizeof(cmd));
cmd[0] = MODE_SENSE;
cmd[1] = ch->device->lun << 5;
@@ -367,7 +367,7 @@ ch_readconfig(scsi_changer *ch)
} else {
vprintk("reading element address assigment page failed!\n");
}
-   
+
/* vendor specific element types */
for (i = 0; i < 4; i++) {
if (0 == vendor_counts[i])
@@ -384,6 +384,10 @@ ch_readconfig(scsi_changer *ch)
/* look up the devices of the data transfer elements */
ch->dt = kmalloc(ch->counts[CHET_DT]*sizeof(struct scsi_device),
 GFP_KERNEL);
+   if (!ch->dt) {
+   kfree(buffer);
+   return -ENOMEM;
+   }
for (elem = 0; elem < ch->counts[CHET_DT]; elem++) {
id  = -1;
lun = 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/videocodec.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
videocodec_build_table(), in file drivers/media/video/videocodec.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/videocodec.c b/drivers/media/video/videocodec.c
index 2ae3fb2..16fc1dd 100644
--- a/drivers/media/video/videocodec.c
+++ b/drivers/media/video/videocodec.c
@@ -348,6 +348,8 @@ #define LINESIZE 100
kfree(videocodec_buf);
videocodec_buf = (char *) kmalloc(size, GFP_KERNEL);
 
+   if (!videocodec_buf)
+   return 0;
i = 0;
i += scnprintf(videocodec_buf + i, size - 1,
  "lave or attached aster name  type flagsmagic   
 ");
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/videocodec.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
videocodec_build_table(), in file drivers/media/video/videocodec.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/videocodec.c b/drivers/media/video/videocodec.c
index 2ae3fb2..16fc1dd 100644
--- a/drivers/media/video/videocodec.c
+++ b/drivers/media/video/videocodec.c
@@ -348,6 +348,8 @@ #define LINESIZE 100
kfree(videocodec_buf);
videocodec_buf = (char *) kmalloc(size, GFP_KERNEL);
 
+   if (!videocodec_buf)
+   return 0;
i = 0;
i += scnprintf(videocodec_buf + i, size - 1,
  Slave or attached Master name  type flagsmagic   
 );
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scsi: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function ch_readconfig(), 
in file drivers/scsi/ch.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index f6caa43..fcd635b 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -324,7 +324,7 @@ ch_readconfig(scsi_changer *ch)
if (!buffer)
return -ENOMEM;
memset(buffer,0,512);
-   
+
memset(cmd,0,sizeof(cmd));
cmd[0] = MODE_SENSE;
cmd[1] = ch-device-lun  5;
@@ -367,7 +367,7 @@ ch_readconfig(scsi_changer *ch)
} else {
vprintk(reading element address assigment page failed!\n);
}
-   
+
/* vendor specific element types */
for (i = 0; i  4; i++) {
if (0 == vendor_counts[i])
@@ -384,6 +384,10 @@ ch_readconfig(scsi_changer *ch)
/* look up the devices of the data transfer elements */
ch-dt = kmalloc(ch-counts[CHET_DT]*sizeof(struct scsi_device),
 GFP_KERNEL);
+   if (!ch-dt) {
+   kfree(buffer);
+   return -ENOMEM;
+   }
for (elem = 0; elem  ch-counts[CHET_DT]; elem++) {
id  = -1;
lun = 0;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs/cifs/readdir.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function cifs_readdir(), in 
file fs/cifs/readdir.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index b5b0a2a..2d43b2a 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -1063,6 +1063,11 @@ int cifs_readdir(struct file *file, void
such multibyte target UTF-8 characters. cifs_unicode.c,
which actually does the conversion, has the same limit */
tmp_buf = kmalloc((2 * NAME_MAX) + 4, GFP_KERNEL);
+   if (!tmp_buf) {
+   cERROR(1, (No memory!));
+   rc = -ENOMEM;
+   goto rddir2_exit;
+   }
for(i=0;(inum_to_fill)  (rc == 0);i++) {
if(current_entry == NULL) {
/* evaluate whether this case is an error */
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/usb/serial/mos7840.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function mos7840_get_reg(), 
in file drivers/usb/serial/mos7840.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 021be39..91d474b 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
int ret = 0;
buffer = (__u8 *) mcs-ctrl_buf;
 
+   /* The memory for ctrl_buf is allocated in
+* mos7840_startup(), but it is not checked if
+* kmalloc failed. So, mcs-ctrl_buf might be NULL.
+* So, it should be checked here.
+*/
+   if (!buffer)
+   return -ENOMEM;
+
 //  dr=(struct usb_ctrlrequest *)(buffer);
dr = (void *)(buffer + 2);
dr-bRequestType = MCS_RD_RTYPE;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..a1ec3ac 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,7 @@ static int stv680_start_stream (struct u
stv680-sbuf[i].data = kmalloc (stv680-rawbufsize, GFP_KERNEL);
if (stv680-sbuf[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw data buffer 
%i, i);
-   return -1;
+   goto nomem_err;
}
}
 
@@ -698,7 +698,7 @@ static int stv680_start_stream (struct u
stv680-scratch[i].data = kmalloc (stv680-rawbufsize, 
GFP_KERNEL);
if (stv680-scratch[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw scratch 
buffer %i, i);
-   return -1;
+   goto nomem_err;
}
stv680-scratch[i].state = BUFFER_UNUSED;
}
@@ -706,7 +706,7 @@ static int stv680_start_stream (struct u
for (i = 0; i  STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
if (!urb)
-   return -ENOMEM;
+   goto nomem_err;
 
/* sbuf is urb-transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680-udev,
@@ -721,6 +721,21 @@ static int stv680_start_stream (struct u
 
stv680-framecount = 0;
return 0;
+
+ nomem_err:
+   for (i = 0; i  STV680_NUMSCRATCH; i++) {
+   kfree(stv680-scratch[i].data);
+   stv680-scratch[i].data = NULL;
+   }
+   for (i = 0; i  STV680_NUMSBUF; i++) {
+   usb_kill_urb(stv680-urb[i]);
+   usb_free_urb(stv680-urb[i]);
+   stv680-urb[i] = NULL;
+   kfree(stv680-sbuf[i].data);
+   stv680-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] drivers/media/video/stv680.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
stv680_start_stream(), in file drivers/media/video/stv680.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/stv680.c b/drivers/media/video/stv680.c
index 6d1ef1e..f35c664 100644
--- a/drivers/media/video/stv680.c
+++ b/drivers/media/video/stv680.c
@@ -687,7 +687,11 @@ static int stv680_start_stream (struct u
stv680-sbuf[i].data = kmalloc (stv680-rawbufsize, GFP_KERNEL);
if (stv680-sbuf[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw data buffer 
%i, i);
-   return -1;
+   for (i = i - 1; i = 0; i--) {
+   kfree(stv680-sbuf[i].data);
+   stv680-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
}
}
 
@@ -698,15 +702,25 @@ static int stv680_start_stream (struct u
stv680-scratch[i].data = kmalloc (stv680-rawbufsize, 
GFP_KERNEL);
if (stv680-scratch[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw scratch 
buffer %i, i);
-   return -1;
+   for (i = i - 1; i = 0; i--) {
+   kfree(stv680-scratch[i].data);
+   stv680-scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
}
stv680-scratch[i].state = BUFFER_UNUSED;
}
 
for (i = 0; i  STV680_NUMSBUF; i++) {
urb = usb_alloc_urb (0, GFP_KERNEL);
-   if (!urb)
-   return -ENOMEM;
+   if (!urb) {
+   for (i = i - 1; i = 0; i--) {
+   usb_kill_urb(stv680-urb[i]);
+   usb_free_urb(stv680-urb[i]);
+   stv680-urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
/* sbuf is urb-transfer_buffer, later gets memcpyed to scratch 
*/
usb_fill_bulk_urb (urb, stv680-udev,
@@ -721,6 +735,18 @@ static int stv680_start_stream (struct u
 
stv680-framecount = 0;
return 0;
+
+ nomem_scratch:
+   for (i=0; iSTV680_NUMSCRATCH; i++) {
+   kfree(stv680-scratch[i].data);
+   stv680-scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; iSTV680_NUMSBUF; i++) {
+   kfree(stv680-sbuf[i].data);
+   stv680-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int stv680_stop_stream (struct usb_stv *stv680)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mm/slab.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function setup_cpu_cache(), 
in file mm/slab.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/mm/slab.c b/mm/slab.c
index 84c631f..613ae61 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2021,6 +2021,7 @@ static int setup_cpu_cache(struct kmem_c
} else {
cachep-array[smp_processor_id()] =
kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+   BUG_ON(!cachep-array[smp_processor_id()]);
 
if (g_cpucache_up == PARTIAL_AC) {
set_up_list3s(cachep, SIZE_L3);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] sound/oss/i810_audio.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function i810_open(), in 
file sound/oss/i810_audio.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/sound/oss/i810_audio.c b/sound/oss/i810_audio.c
index 240cc79..a415967 100644
--- a/sound/oss/i810_audio.c
+++ b/sound/oss/i810_audio.c
@@ -2580,8 +2580,13 @@ static int i810_open(struct inode *inode
if (card-states[i] == NULL) {
state = card-states[i] = (struct i810_state *)
kmalloc(sizeof(struct i810_state), 
GFP_KERNEL);
-   if (state == NULL)
+   if (state == NULL) {
+   for (--i; i = 0; i--) {
+   kfree(card-states[i]);
+   card-states[i] = NULL;
+   }
return -ENOMEM;
+   }
memset(state, 0, sizeof(struct i810_state));
dmabuf = state-dmabuf;
goto found_virt;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/atm/fore200e.c: change in error message.

[Amit Choudhary]

Description: Change in error message in function fore200e_kmalloc(), in file 
drivers/atm/fore200e.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 3a7b21f..1c7ea02 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -178,7 +178,7 @@ fore200e_kmalloc(int size, gfp_t flags)
 void *chunk = kzalloc(size, flags);
 
 if (!chunk)
-   printk(FORE200E kmalloc() failed, requested size = %d, flags = 
0x%x\n,size, flags);
+   printk(FORE200E kzalloc() failed, requested size = %d, flags = 
0x%x\n,size, flags);
 
 return chunk;
 }
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] net/wanrouter/wanmain.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function dbg_kmalloc(), in 
file net/wanrouter/wanmain.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/net/wanrouter/wanmain.c b/net/wanrouter/wanmain.c
index 316211d..263450c 100644
--- a/net/wanrouter/wanmain.c
+++ b/net/wanrouter/wanmain.c
@@ -67,6 +67,8 @@ static void * dbg_kmalloc(unsigned int s
int i = 0;
void * v = kmalloc(size+sizeof(unsigned int)+2*KMEM_SAFETYZONE*8,prio);
char * c1 = v;
+   if (!v)
+   return NULL;
c1 += sizeof(unsigned int);
*((unsigned int *)v) = size;
 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/vt.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function con_init(), in 
file drivers/char/vt.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index 87587b4..6aa08cb 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -2640,6 +2640,15 @@ static int __init con_init(void)
 */
for (currcons = 0; currcons  MIN_NR_CONSOLES; currcons++) {
vc_cons[currcons].d = vc = alloc_bootmem(sizeof(struct 
vc_data));
+   if (!vc_cons[currcons].d) {
+   for (--currcons; currcons = 0; currcons--) {
+   kfree(vc_cons[currcons].d);
+   vc_cons[currcons].d = NULL;
+   }
+   release_console_sem();
+   return -ENOMEM;
+   }
+
visual_init(vc, currcons, 1);
vc-vc_screenbuf = (unsigned short 
*)alloc_bootmem(vc-vc_screenbuf_size);
vc-vc_kmalloced = 0;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] fs/jffs2/scan.c: Fix error-path leak

[Amit Choudhary]

Description: Fix error-path leak in function jffs2_scan_medium(), in file 
fs/jffs2/scan.c

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index e241346..cd9ed6e 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -130,6 +130,8 @@ #endif
if (jffs2_sum_active()) {
s = kmalloc(sizeof(struct jffs2_summary), GFP_KERNEL);
if (!s) {
+   free(flashbuf);
+   flashbuf = NULL;
JFFS2_WARNING(Can't allocate memory for summary\n);
return -ENOMEM;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/agp/sgi-agp.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function agp_sgi_init(), in 
file drivers/char/agp/sgi-agp.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c
index d73be4c..5897e6c 100644
--- a/drivers/char/agp/sgi-agp.c
+++ b/drivers/char/agp/sgi-agp.c
@@ -285,6 +285,8 @@ static int __devinit agp_sgi_init(void)
(struct agp_bridge_data **)kmalloc(tioca_gart_found *
   sizeof(struct agp_bridge_data *),
   GFP_KERNEL);
+   if (!sgi_tioca_agp_bridges)
+   return -ENOMEM;
 
j = 0;
list_for_each_entry(info, tioca_list, ca_list) {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/media/video/se401.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
se401_start_stream(), in file drivers/media/video/se401.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/se401.c b/drivers/media/video/se401.c
index 7aeec57..006c818 100644
--- a/drivers/media/video/se401.c
+++ b/drivers/media/video/se401.c
@@ -450,6 +450,13 @@ static int se401_start_stream(struct usb
}
for (i=0; iSE401_NUMSBUF; i++) {
se401-sbuf[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401-sbuf[i].data) {
+   for(i = i - 1; i = 0; i--) {
+   kfree(se401-sbuf[i].data);
+   se401-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
+   }
}
 
se401-bayeroffset=0;
@@ -458,13 +465,26 @@ static int se401_start_stream(struct usb
se401-scratch_overflow=0;
for (i=0; iSE401_NUMSCRATCH; i++) {
se401-scratch[i].data=kmalloc(SE401_PACKETSIZE, GFP_KERNEL);
+   if (!se401-scratch[i].data) {
+   for(i = i - 1; i = 0; i--) {
+   kfree(se401-scratch[i].data);
+   se401-scratch[i].data = NULL;
+   }
+   goto nomem_sbuf;
+   }
se401-scratch[i].state=BUFFER_UNUSED;
}
 
for (i=0; iSE401_NUMSBUF; i++) {
urb=usb_alloc_urb(0, GFP_KERNEL);
-   if(!urb)
-   return -ENOMEM;
+   if(!urb) {
+   for(i = i - 1; i = 0; i--) {
+   usb_kill_urb(se401-urb[i]);
+   usb_free_urb(se401-urb[i]);
+   se401-urb[i] = NULL;
+   }
+   goto nomem_scratch;
+   }
 
usb_fill_bulk_urb(urb, se401-dev,
usb_rcvbulkpipe(se401-dev, SE401_VIDEO_ENDPOINT),
@@ -482,6 +502,18 @@ static int se401_start_stream(struct usb
se401-framecount=0;
 
return 0;
+
+ nomem_scratch:
+   for (i=0; iSE401_NUMSCRATCH; i++) {
+   kfree(se401-scratch[i].data);
+   se401-scratch[i].data = NULL;
+   }
+ nomem_sbuf:
+   for (i=0; iSE401_NUMSBUF; i++) {
+   kfree(se401-sbuf[i].data);
+   se401-sbuf[i].data = NULL;
+   }
+   return -ENOMEM;
 }
 
 static int se401_stop_stream(struct usb_se401 *se401)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [REVISED] net/ipv4/multipath_wrandom.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
wrandom_set_nhinfo(), in file net/ipv4/multipath_wrandom.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/net/ipv4/multipath_wrandom.c b/net/ipv4/multipath_wrandom.c
index 92b0482..bcdb1f1 100644
--- a/net/ipv4/multipath_wrandom.c
+++ b/net/ipv4/multipath_wrandom.c
@@ -242,6 +242,9 @@ static void wrandom_set_nhinfo(__be32 ne
target_route = (struct multipath_route *)
kmalloc(size_rt, GFP_ATOMIC);
 
+   if (!target_route)
+   goto error;
+
target_route-gw = nh-nh_gw;
target_route-oif = nh-nh_oif;
memset(target_route-rcu, 0, sizeof(struct rcu_head));
@@ -263,6 +266,9 @@ static void wrandom_set_nhinfo(__be32 ne
target_dest = (struct multipath_dest*)
kmalloc(size_dst, GFP_ATOMIC);
 
+   if (!target_dest)
+   goto error;
+
target_dest-nh_info = nh;
target_dest-network = network;
target_dest-netmask = netmask;
@@ -275,6 +281,7 @@ static void wrandom_set_nhinfo(__be32 ne
 * we are finished
 */
 
+ error:
spin_unlock_bh(state[state_idx].lock);
 }
 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/char/synclink.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function 
mgsl_alloc_intermediate_txbuffer_memory(), in file drivers/char/synclink.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c
index 06784ad..24f99bc 100644
--- a/drivers/char/synclink.c
+++ b/drivers/char/synclink.c
@@ -4012,8 +4012,13 @@ static int mgsl_alloc_intermediate_txbuf
for ( i=0; iinfo-num_tx_holding_buffers; ++i) {
info-tx_holding_buffers[i].buffer =
kmalloc(info-max_frame_size, GFP_KERNEL);
-   if ( info-tx_holding_buffers[i].buffer == NULL )
+   if (info-tx_holding_buffers[i].buffer == NULL) {
+   for (--i; i = 0; i--) {
+   kfree(info-tx_holding_buffers[i].buffer);
+   info-tx_holding_buffers[i].buffer = NULL;
+   }
return -ENOMEM;
+   }
}
 
return 0;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Randy Dunlap <[EMAIL PROTECTED]> wrote:

> On Tue, 9 Jan 2007 11:02:35 -0800 (PST) Amit Choudhary wrote:
> 
> No thanks.  If a driver author wants to maintain driver state
> that way, it's OK, but that doesn't make it a global requirement.
> 

Ok. So, a driver can have its own local definition of KFREE() macro.

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- [EMAIL PROTECTED] wrote:

> On Tue, 09 Jan 2007 11:02:35 PST, Amit Choudhary said:
> > Correct. And doing kfree(x); x=NULL; is not hiding that. These issues can 
> > still be debugged by
> > using the slab debugging options. One other benefit of doing this is that 
> > if someone tries to
> > access the same memory again using the variable 'x', then he will get an 
> > immediate crash.


What did you understand when I wrote that "if you access the same memory again 
using the variable
'x"?

Using variable 'x' means using variable 'x' and not variable 'y'.


 And
> the
> > problem can be solved immediately, without using the slab debugging 
> > options. I do not yet
> > understand how doing this hides the bugs, obfuscates the code, etc. because 
> > I haven't seen an
> > example yet, but only blanket statements.
> 
> char *broken() {
>   char *x, *y;
>   x = kmalloc(100);
>   y = x;
>   kfree(x);
>   x = NULL;
>   return y;
> }
> 
> Setting x to NULL doesn't do anything to fix the *real* bug here, because
> the problematic reference is held in y, not x.  


Did I ever say that it fixes that kind of bug?


>So you never get a crash
> because somebody dereferences x.
> 


Dereferencing 'x' means dereferencing 'x' and not dereferencing 'y'.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- [EMAIL PROTECTED] wrote:

> On Mon, 08 Jan 2007 01:06:12 PST, Amit Choudhary said:
> > I do not see how a double free can result in _logical_wrong_behaviour_ of 
> > the program and the
> > program keeps on running (like an incoming packet being dropped because of 
> > double free).
> Double
> > free will _only_and_only_ result in system crash that can be solved by 
> > setting 'x' to NULL.
> 
> The problem is that very rarely is there a second free() with no intervening
> use - what actually *happens* usually is:
> 
> 1) You alloc the memory
> 2) You use the memory
> 3) You take a reference on the memory, so you know where it is.
> 4) You free the memory
> 5) You use the memory via the reference you took in (3)
> 6) You free it again - at which point you finally know for sure that
> everything in step 5 was doing a fandango on core
> 

Correct. And doing kfree(x); x=NULL; is not hiding that. These issues can still 
be debugged by
using the slab debugging options. One other benefit of doing this is that if 
someone tries to
access the same memory again using the variable 'x', then he will get an 
immediate crash. And the
problem can be solved immediately, without using the slab debugging options. I 
do not yet
understand how doing this hides the bugs, obfuscates the code, etc. because I 
haven't seen an
example yet, but only blanket statements.

But now I know better, since I haven't heard anything in support of this case, 
I have concluded
that doing kfree(x); x=NULL; is _not_needed_ in the "linux kernel". I hope that 
no one does it in
the future. And since people vehemently opposed this, I think its better to add 
another item on
the kernel janitor's list to remove all the (x=NULL) statements where people 
are doing "kfree(x);
x=NULL".

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- [EMAIL PROTECTED] wrote:

 On Mon, 08 Jan 2007 01:06:12 PST, Amit Choudhary said:
  I do not see how a double free can result in _logical_wrong_behaviour_ of 
  the program and the
  program keeps on running (like an incoming packet being dropped because of 
  double free).
 Double
  free will _only_and_only_ result in system crash that can be solved by 
  setting 'x' to NULL.
 
 The problem is that very rarely is there a second free() with no intervening
 use - what actually *happens* usually is:
 
 1) You alloc the memory
 2) You use the memory
 3) You take a reference on the memory, so you know where it is.
 4) You free the memory
 5) You use the memory via the reference you took in (3)
 6) You free it again - at which point you finally know for sure that
 everything in step 5 was doing a fandango on core
 

Correct. And doing kfree(x); x=NULL; is not hiding that. These issues can still 
be debugged by
using the slab debugging options. One other benefit of doing this is that if 
someone tries to
access the same memory again using the variable 'x', then he will get an 
immediate crash. And the
problem can be solved immediately, without using the slab debugging options. I 
do not yet
understand how doing this hides the bugs, obfuscates the code, etc. because I 
haven't seen an
example yet, but only blanket statements.

But now I know better, since I haven't heard anything in support of this case, 
I have concluded
that doing kfree(x); x=NULL; is _not_needed_ in the linux kernel. I hope that 
no one does it in
the future. And since people vehemently opposed this, I think its better to add 
another item on
the kernel janitor's list to remove all the (x=NULL) statements where people 
are doing kfree(x);
x=NULL.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- [EMAIL PROTECTED] wrote:

 On Tue, 09 Jan 2007 11:02:35 PST, Amit Choudhary said:
  Correct. And doing kfree(x); x=NULL; is not hiding that. These issues can 
  still be debugged by
  using the slab debugging options. One other benefit of doing this is that 
  if someone tries to
  access the same memory again using the variable 'x', then he will get an 
  immediate crash.


What did you understand when I wrote that if you access the same memory again 
using the variable
'x?

Using variable 'x' means using variable 'x' and not variable 'y'.


 And
 the
  problem can be solved immediately, without using the slab debugging 
  options. I do not yet
  understand how doing this hides the bugs, obfuscates the code, etc. because 
  I haven't seen an
  example yet, but only blanket statements.
 
 char *broken() {
   char *x, *y;
   x = kmalloc(100);
   y = x;
   kfree(x);
   x = NULL;
   return y;
 }
 
 Setting x to NULL doesn't do anything to fix the *real* bug here, because
 the problematic reference is held in y, not x.  


Did I ever say that it fixes that kind of bug?


So you never get a crash
 because somebody dereferences x.
 


Dereferencing 'x' means dereferencing 'x' and not dereferencing 'y'.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Randy Dunlap [EMAIL PROTECTED] wrote:

 On Tue, 9 Jan 2007 11:02:35 -0800 (PST) Amit Choudhary wrote:
 
 No thanks.  If a driver author wants to maintain driver state
 that way, it's OK, but that doesn't make it a global requirement.
 

Ok. So, a driver can have its own local definition of KFREE() macro.

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Pekka Enberg <[EMAIL PROTECTED]> wrote:

> Hi Amit,
> 
> On 1/8/07, Amit Choudhary <[EMAIL PROTECTED]> wrote:
> > Man, doesn't make sense to me.
> 
> Well, man, double-free is a programming error and papering over it
> with NULL initializations bloats the kernel and makes the code
> confusing.
> 
> Clear enough for you?
> 

It is a programming error because the underlying code cannot handle it. If, 
from the beginning of
time, double free would have been handled properly then we wouldn't have 
thought twice about it.

You want to catch double frees. What if double frees are no-ops?

I do not see how a double free can result in _logical_wrong_behaviour_ of the 
program and the
program keeps on running (like an incoming packet being dropped because of 
double free). Double
free will _only_and_only_ result in system crash that can be solved by setting 
'x' to NULL.

-Amit



__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Sumit Narayan <[EMAIL PROTECTED]> wrote:

> Asking for KFREE is as silly as asking for a macro to check if kmalloc
> succeeded for a pointer, else return ENOMEM.
> 
> #define CKMALLOC(p,x) \
>do {   \
>p = kmalloc(x, GFP_KERNEL); \
>if(!p) return -ENOMEM; \
> } while(0)
> 

There are bugs with this approach. This introduces error path leaks. If you 
have allocated some
memory earlier, then you got to free them.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov <[EMAIL PROTECTED]> wrote:

> On Sun, 2007-01-07 at 23:29 -0800, Amit Choudhary wrote:
> > I do not want to write this but I think that you are arguing just for the 
> > heck of it. Please
> be
> > sane.
> 
> No, I'm merely trying to demonstrate, on a logical basis, why the
> proposed patch does not (in my opinion) belong within the kernel. The
> fact that I'm not alone in voicing such disagreement should mean
> something.
> 

I agree that since couple of people are voicing disagreement the definitely it 
means something and
probably it means that you are right.

Let's try to apply the same logic to my explanation:

KFREE() macro has __actually__ been used at atleast 1000 places in the kernel 
by atleast 50
different people. Doesn't that lend enough credibility to what I am saying.

People did something like this 1000 times: kfree(x), x = NULL. I simply 
proposed the KFREE() macro
that does the same thing. Resistance to something that is already being done in 
the kernel. I
really do not care whether it goes in the kernel or not. There are lots of 
other places where I
can contribute. But I do not understand the resistance.

It is already being done in the kernel.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Pekka Enberg <[EMAIL PROTECTED]> wrote:

> On 1/8/07, Hua Zhong <[EMAIL PROTECTED]> wrote:
> > > And as I explained, it can result in longer code too. So, why
> > > keep this value around. Why not re-initialize it to NULL.
> >
> > Because initialization increases code size.
> 
> And it also effectively blocks the slab debugging code from doing its
> job detecting double-frees.
> 

Man, so you do want someone to set 'x' to NULL after freeing it, so that the 
slab debugging code
can catch double frees. If you set it to NULL then double free is harmless. So, 
you want something
harmful in the system and then debug it with the slab debugging code. Man, 
doesn't make sense to
me.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Hua Zhong <[EMAIL PROTECTED]> wrote:

> > Any strong reason why not? x has some value that does not 
> > make sense and can create only problems.
> 
> By the same logic, you should memset the buffer to zero before freeing it too.
> 

How does this help?

> > And as I explained, it can result in longer code too. So, why 
> > keep this value around. Why not re-initialize it to NULL.
> 
> Because initialization increases code size.

Then why use kzalloc()? Let's remove _ALL_ the initialization code from the 
kernel.

Attached is some code from the kernel. Expanded KFREE() has been used atleast 
1000 times in the
kernel. By your logic, everyone is stupid in doing so. Something has been done 
atleast 1000 times
in the kernel, that looks okay. But consolidating it at one place does not look 
okay. I am listing
some of the 1000 places where KFREE() has been used. All this code have been 
written by atleast 50
different people. From your logic they were doing "silly" things.

--
arch/ppc/kernel/smp-tbsync.c:   kfree( tbsync );
arch/ppc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/powerpc/kernel/smp-tbsync.c:   kfree(tbsync);
arch/powerpc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/powerpc/kernel/rtas_flash.c:   kfree(dp->data);
arch/powerpc/kernel/rtas_flash.c-   dp->data = NULL;
--
arch/powerpc/platforms/ps3/spu.c:   kfree(spu->pdata);
arch/powerpc/platforms/ps3/spu.c-   spu->pdata = NULL;
--
arch/powerpc/platforms/cell/spu_priv1_mmio.c:   kfree(spu->pdata);
arch/powerpc/platforms/cell/spu_priv1_mmio.c-   spu->pdata = NULL;
--
arch/powerpc/platforms/cell/spu_priv1_mmio.c:   kfree(spu->pdata);
arch/powerpc/platforms/cell/spu_priv1_mmio.c-   spu->pdata = NULL;
--
arch/powerpc/platforms/cell/spufs/context.c:kfree(ctx);
arch/powerpc/platforms/cell/spufs/context.c-ctx = NULL;
--
arch/ia64/kernel/topology.c:kfree(all_cpu_cache_info[cpu].cache_leaves);
arch/ia64/kernel/topology.c-all_cpu_cache_info[cpu].cache_leaves = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  
kfree(ch->local_msgqueue_base);
arch/ia64/sn/kernel/xpc_channel.c-  ch->local_msgqueue = 
NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(ch->notify_queue);
arch/ia64/sn/kernel/xpc_channel.c-  ch->notify_queue = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(ch->notify_queue);
arch/ia64/sn/kernel/xpc_channel.c-  ch->notify_queue = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part->channels);
arch/ia64/sn/kernel/xpc_channel.c-  part->channels = NULL;
--
arch/s390/hypfs/inode.c:kfree(sb->s_fs_info);
arch/s390/hypfs/inode.c-sb->s_fs_info = NULL;
--
arch/s390/kernel/debug.c:   kfree(db_info->areas);
arch/s390/kernel/debug.c-   db_info->areas = NULL;
--
arch/sparc64/kernel/of_device.c:kfree(op);
arch/sparc64/kernel/of_device.c-op = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(us2e_freq_table);
arch/sparc64/kernel/us2e_cpufreq.c- us2e_freq_table = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(cpufreq_us2e_driver);
arch/sparc64/kernel/us2e_cpufreq.c- cpufreq_us2e_driver = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(us2e_freq_table);
arch/sparc64/kernel/us2e_cpufreq.c- us2e_freq_table = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(us3_freq_table);
arch/sparc64/kernel/us3_cpufreq.c-  us3_freq_table = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(cpufreq_us3_driver);
arch/sparc64/kernel/us3_cpufreq.c-  cpufreq_us3_driver = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(us3_freq_table);
arch/sparc64/kernel/us3_cpufreq.c-  us3_freq_table = NULL;
--
arch/x86_64/kernel/mce_amd.c:   kfree(per_cpu(threshold_banks, 
cpu)[bank]->blocks);
arch/x86_64/kernel/mce_amd.c-   per_cpu(threshold_banks, cpu)[bank]->blocks = 
NULL;
--
arch/x86_64/kernel/process.c:   kfree(t->io_bitmap_ptr);
arch/x86_64/kernel/process.c-   t->io_bitmap_ptr = NULL;
--
arch/x86_64/kernel/io_apic.c:   kfree(mp_ioapic_data[i]);

RE: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Hua Zhong [EMAIL PROTECTED] wrote:

  Any strong reason why not? x has some value that does not 
  make sense and can create only problems.
 
 By the same logic, you should memset the buffer to zero before freeing it too.
 

How does this help?

  And as I explained, it can result in longer code too. So, why 
  keep this value around. Why not re-initialize it to NULL.
 
 Because initialization increases code size.

Then why use kzalloc()? Let's remove _ALL_ the initialization code from the 
kernel.

Attached is some code from the kernel. Expanded KFREE() has been used atleast 
1000 times in the
kernel. By your logic, everyone is stupid in doing so. Something has been done 
atleast 1000 times
in the kernel, that looks okay. But consolidating it at one place does not look 
okay. I am listing
some of the 1000 places where KFREE() has been used. All this code have been 
written by atleast 50
different people. From your logic they were doing silly things.

--
arch/ppc/kernel/smp-tbsync.c:   kfree( tbsync );
arch/ppc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/powerpc/kernel/smp-tbsync.c:   kfree(tbsync);
arch/powerpc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/powerpc/kernel/rtas_flash.c:   kfree(dp-data);
arch/powerpc/kernel/rtas_flash.c-   dp-data = NULL;
--
arch/powerpc/platforms/ps3/spu.c:   kfree(spu-pdata);
arch/powerpc/platforms/ps3/spu.c-   spu-pdata = NULL;
--
arch/powerpc/platforms/cell/spu_priv1_mmio.c:   kfree(spu-pdata);
arch/powerpc/platforms/cell/spu_priv1_mmio.c-   spu-pdata = NULL;
--
arch/powerpc/platforms/cell/spu_priv1_mmio.c:   kfree(spu-pdata);
arch/powerpc/platforms/cell/spu_priv1_mmio.c-   spu-pdata = NULL;
--
arch/powerpc/platforms/cell/spufs/context.c:kfree(ctx);
arch/powerpc/platforms/cell/spufs/context.c-ctx = NULL;
--
arch/ia64/kernel/topology.c:kfree(all_cpu_cache_info[cpu].cache_leaves);
arch/ia64/kernel/topology.c-all_cpu_cache_info[cpu].cache_leaves = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  
kfree(ch-local_msgqueue_base);
arch/ia64/sn/kernel/xpc_channel.c-  ch-local_msgqueue = 
NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(ch-notify_queue);
arch/ia64/sn/kernel/xpc_channel.c-  ch-notify_queue = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(ch-notify_queue);
arch/ia64/sn/kernel/xpc_channel.c-  ch-notify_queue = NULL;
--
arch/ia64/sn/kernel/xpc_channel.c:  kfree(part-channels);
arch/ia64/sn/kernel/xpc_channel.c-  part-channels = NULL;
--
arch/s390/hypfs/inode.c:kfree(sb-s_fs_info);
arch/s390/hypfs/inode.c-sb-s_fs_info = NULL;
--
arch/s390/kernel/debug.c:   kfree(db_info-areas);
arch/s390/kernel/debug.c-   db_info-areas = NULL;
--
arch/sparc64/kernel/of_device.c:kfree(op);
arch/sparc64/kernel/of_device.c-op = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(us2e_freq_table);
arch/sparc64/kernel/us2e_cpufreq.c- us2e_freq_table = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(cpufreq_us2e_driver);
arch/sparc64/kernel/us2e_cpufreq.c- cpufreq_us2e_driver = NULL;
--
arch/sparc64/kernel/us2e_cpufreq.c: kfree(us2e_freq_table);
arch/sparc64/kernel/us2e_cpufreq.c- us2e_freq_table = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(us3_freq_table);
arch/sparc64/kernel/us3_cpufreq.c-  us3_freq_table = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(cpufreq_us3_driver);
arch/sparc64/kernel/us3_cpufreq.c-  cpufreq_us3_driver = NULL;
--
arch/sparc64/kernel/us3_cpufreq.c:  kfree(us3_freq_table);
arch/sparc64/kernel/us3_cpufreq.c-  us3_freq_table = NULL;
--
arch/x86_64/kernel/mce_amd.c:   kfree(per_cpu(threshold_banks, 
cpu)[bank]-blocks);
arch/x86_64/kernel/mce_amd.c-   per_cpu(threshold_banks, cpu)[bank]-blocks = 
NULL;
--
arch/x86_64/kernel/process.c:   kfree(t-io_bitmap_ptr);
arch/x86_64/kernel/process.c-   t-io_bitmap_ptr = NULL;
--
arch/x86_64/kernel/io_apic.c:   kfree(mp_ioapic_data[i]);
arch/x86_64/kernel/io_apic.c-   mp_ioapic_data[i] = NULL;
--

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Pekka Enberg [EMAIL PROTECTED] wrote:

 On 1/8/07, Hua Zhong [EMAIL PROTECTED] wrote:
   And as I explained, it can result in longer code too. So, why
   keep this value around. Why not re-initialize it to NULL.
 
  Because initialization increases code size.
 
 And it also effectively blocks the slab debugging code from doing its
 job detecting double-frees.
 

Man, so you do want someone to set 'x' to NULL after freeing it, so that the 
slab debugging code
can catch double frees. If you set it to NULL then double free is harmless. So, 
you want something
harmful in the system and then debug it with the slab debugging code. Man, 
doesn't make sense to
me.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov [EMAIL PROTECTED] wrote:

 On Sun, 2007-01-07 at 23:29 -0800, Amit Choudhary wrote:
  I do not want to write this but I think that you are arguing just for the 
  heck of it. Please
 be
  sane.
 
 No, I'm merely trying to demonstrate, on a logical basis, why the
 proposed patch does not (in my opinion) belong within the kernel. The
 fact that I'm not alone in voicing such disagreement should mean
 something.
 

I agree that since couple of people are voicing disagreement the definitely it 
means something and
probably it means that you are right.

Let's try to apply the same logic to my explanation:

KFREE() macro has __actually__ been used at atleast 1000 places in the kernel 
by atleast 50
different people. Doesn't that lend enough credibility to what I am saying.

People did something like this 1000 times: kfree(x), x = NULL. I simply 
proposed the KFREE() macro
that does the same thing. Resistance to something that is already being done in 
the kernel. I
really do not care whether it goes in the kernel or not. There are lots of 
other places where I
can contribute. But I do not understand the resistance.

It is already being done in the kernel.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Sumit Narayan [EMAIL PROTECTED] wrote:

 Asking for KFREE is as silly as asking for a macro to check if kmalloc
 succeeded for a pointer, else return ENOMEM.
 
 #define CKMALLOC(p,x) \
do {   \
p = kmalloc(x, GFP_KERNEL); \
if(!p) return -ENOMEM; \
 } while(0)
 

There are bugs with this approach. This introduces error path leaks. If you 
have allocated some
memory earlier, then you got to free them.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Pekka Enberg [EMAIL PROTECTED] wrote:

 Hi Amit,
 
 On 1/8/07, Amit Choudhary [EMAIL PROTECTED] wrote:
  Man, doesn't make sense to me.
 
 Well, man, double-free is a programming error and papering over it
 with NULL initializations bloats the kernel and makes the code
 confusing.
 
 Clear enough for you?
 

It is a programming error because the underlying code cannot handle it. If, 
from the beginning of
time, double free would have been handled properly then we wouldn't have 
thought twice about it.

You want to catch double frees. What if double frees are no-ops?

I do not see how a double free can result in _logical_wrong_behaviour_ of the 
program and the
program keeps on running (like an incoming packet being dropped because of 
double free). Double
free will _only_and_only_ result in system crash that can be solved by setting 
'x' to NULL.

-Amit



__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov <[EMAIL PROTECTED]> wrote:

> On Sun, 2007-01-07 at 20:09 -0800, Amit Choudhary wrote:
> > I have already explained it earlier. I will try again. You will not need 
> > free_2: and free_1:
> with
> > KFREE(). You will only need one free: with KFREE.
> 

I do not want to write this but I think that you are arguing just for the heck 
of it. Please be
sane.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov <[EMAIL PROTECTED]> wrote:

> 
> struct type1 {
>   /* something */
> };
> 
> struct type2 {
>   /* something */
> };
> 
> #define COUNT 10
> 
> void function1(struct type1 **a1, struct type2 **a2, unsigned int sz);
> 
> void function2(void)
> {
>   struct type1 *arr1[COUNT];
>   struct type2 *arr2[COUNT];
>   int i;
> 
>   /* init */
>   for (i = 0; i < COUNT; i++) {
>   arr1[i] = kmalloc(sizeof(struct type1), ...);
>   if (!arr1[i])
>   goto free_1;
>   }
>   for (i = 0; i < COUNT; i++) {
>   arr2[i] = kmalloc(sizeof(struct type2), ...);
>   if (!arr2[i])
>   goto free_2;
>   }
> 
>   /* work */
>   function1(arr1, arr2, COUNT);
> 
>   /* fini */
>   i = COUNT;
> free_2:
>   for (i--; i >= 0; i--) {
>   kfree(arr2[i]);
>   }
>   i = COUNT;
> free_1:
>   for (i--; i >= 0; i--) {
>   kfree(arr1[i]);
>   }
> }
> 
> In most cases, though, the above code design would be brain-damaged from
> the start: unless the sizes involved are prohibitively large, the
> function should be allocating all the memory in a single pass.
> 
> So, where's the demonstrated need for KFREE()?

I have already explained it earlier. I will try again. You will not need 
free_2: and free_1: with
KFREE(). You will only need one free: with KFREE.

Also, let's say that count is different for each array? Then how do you propose 
that memory be
allocated in one pass?

>In most cases, though, the above code design would be brain-damaged from
>the start: unless the sizes involved are prohibitively large, the
>function should be allocating all the memory in a single pass.

Well, only if everything would be fine and correct, we would not be needing 
anything. If you think
this kind of code is brain-damaged then the linux kernel has couple of these.

I have scanned the whole kernel to check whether people are checking for return 
values of kmalloc,
I found that at many places they don't and have sent patches for them. Now, 
this too is brain
damaged code. And during the scan I saw examples of what I described earlier.

KFREE() can fix some of those cases.

Consider this as the proof of what I explained earlier. This fucntion was 
broken but I fixed it
and then realized why KFREE() is needed. 2 kmallocs and 1 usb_alloc_urb(). 
Well, I can only give
examples why KFREE() is needed. If you do not agree, I cannot force you to 
agree with me. And if
you really do not want to agree then even my examples will fail. Also, if you 
think that people
are not doing KFREE() kind of stuff then you should scan the kernel and you 
will see it for
yourself.


Below are some examples where people are doing KFREE() kind of stuff:

--
arch/ppc/kernel/smp-tbsync.c:   kfree( tbsync );
arch/ppc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/ppc/8260_io/fcc_enet.c:
dev_kfree_skb(fep->tx_skbuff[i]);
arch/ppc/8260_io/fcc_enet.c-fep->tx_skbuff[i] = NULL;
--
arch/ppc/8xx_io/cs4218_tdm.c:   kfree (sound_buffers);
arch/ppc/8xx_io/cs4218_tdm.c-   sound_buffers = 0;
--
arch/ia64/kernel/topology.c:kfree(all_cpu_cache_info[cpu].cache_leaves);
arch/ia64/kernel/topology.c-all_cpu_cache_info[cpu].cache_leaves = NULL;
--
arch/ia64/kernel/acpi.c:kfree(buffer.pointer);
arch/ia64/kernel/acpi.c-buffer.pointer = NULL;
--
arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c:
kfree(acpi_perf_data[j]);
arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c-
acpi_perf_data[j] = NULL;
--

There are many more and you can scan the kernel yourself.

Below is where memory is allocated in different arrays with different counts:

static int stv680_start_stream (struct usb_stv *stv680)
{
struct urb *urb;
int err = 0, i;

stv680->streaming = 1;

/* Do some memory allocation */
for (i = 0; i < STV680_NUMFRAMES; i++) {
stv680->frame[i].data = stv680->fbuf + i * stv680->maxframesize;
stv680->frame[i].curpix = 0;
}
/* packet size = 4096  */
for (i = 0; i < STV680_NUMSBUF; i++) {
stv680->sbuf[i].data = kmalloc (stv680->rawbufsize, GFP_KERNEL);
if (stv680->sbuf[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw data buffer 
%i", i);
goto nomem_err;
}
}

stv680->scratch_next = 0;
stv680->scratch_use = 0;
stv680->scratch_overflow = 0;
for (i = 0; i < STV680_NUMSCRATCH; i++) {
stv680->scratch[i].data = kmalloc (stv680->rawbufsize, 
GFP_KERNEL);
if (stv680->scratch[i].data == NULL) {
PDEBUG (0, "STV(e): Could not kmalloc raw scratch 
buffer %i", i);
goto 

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov <[EMAIL PROTECTED]> wrote:

> On Sun, 2007-01-07 at 14:43 -0800, Amit Choudhary wrote:
> > Any strong reason why not? x has some value that does not make sense and 
> > can create only
> problems.
> > And as I explained, it can result in longer code too. So, why keep this 
> > value around. Why not
> > re-initialize it to NULL.
> 
> Because it looks really STRANGE(tm). Consider the following function,
> which is essentially what you're proposing in macro-ized form:
>   void foobar(void)
>   {
>   void *ptr;
> 
>   ptr = kmalloc(...);
>   // actual work here
>   kfree(ptr);
>   ptr = NULL;
>   }

That's where KFREE(ptr) comes in so that the code doesn't look ugly and still 
the purpose is
achieved.

"I still do not know of a single good reason as to why we should not do this."

And if all programmers did the right thing always then why do we have all the 
debugging options in
the first place.

> Reading code like that makes me say "wtf?", simply because 'ptr' is not
> used thereafter,

Really? Then why do we have all the debugging options to catch re-use of the 
memory that has been
freed. So many debugging options has been implemented, so much effort has gone 
into them, partly
because programmers sometimes miss correct programming.

> so setting it to NULL is both pointless and confusing
> (it looks out-of-place, and therefore makes me wonder if there's
> something stupidly tricky going on).
> 
> Also, arguably, your demonstration of why the lack of the proposed
> KFREE() macro results in longer code is invalid. Whereas you wrote:
>   pointer *arr_x[size_x];
>   pointer *arr_y[size_y];
>   pointer *arr_z[size_z];
> That really should have been:
>   pointer *arr[size_x + size_y + size_z];
> or:
>   pointer **arr[3] = { arr_x, arr_y, arr_z };
> In which case, the you only need one path in the function to handle
> allocation failures, rather than the three that you were arguing for.
> 

I do not know what you are talking about here. You are saying that a function 
does not need three
different arrays with different names. How can you say that? How do you know 
what is the
requirement?

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Christoph Hellwig <[EMAIL PROTECTED]> wrote:

> On Sun, Jan 07, 2007 at 12:46:50AM -0800, Amit Choudhary wrote:
> > Well, I am not proposing this as a debugging aid. The idea is about correct 
> > programming,
> atleast
> > from my view. Ideally, if you kfree(x), then you should set x to NULL. So, 
> > either programmers
> do
> > it themselves or a ready made macro do it for them.
> 
> No, you should not.  I suspect that's the basic point you're missing.
> 
> 

Any strong reason why not? x has some value that does not make sense and can 
create only problems.
And as I explained, it can result in longer code too. So, why keep this value 
around. Why not
re-initialize it to NULL.

If x should not be re-initialized to NULL, then by the same logic, we should 
not even initialize
local variables. And all of us know that local variables should be initialized.

I would like to know a good reason as to why x should not be set to NULL.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [DISCUSS] Making system calls more portable.

[Amit Choudhary]

--- Rene Herman <[EMAIL PROTECTED]> wrote:
>
>If we're limited to Linux kernels, this seems to not be the case. Great care 
>is taken in keeping
>this userspace ABI stable -- new system calls are given new numbers. Old 
>system calls may
>disappear (after a long grace period) but even then I don't believe the number 
>is ever recycled.
>
> If your discussion is not limited to Linux kernels, then sure, but being 
> portable at that (sub-libc) level is asking too much.
> 

I will come to the main issue later but I just wanted to point out that we 
maintain information at
two separate places - mapping between the name and the number in user space and 
kernel space.
Shouldn't this duplication be removed.

Now, let's say a vendor has linux_kernel_version_1 that has 300 system calls. 
The vendor needs to
give some extra functionality to its customers and the way chosen is to 
implement new system call.
 The new system call number is 301. The customer gets this custom kernel and 
uses number 301.
Next, he downloads another kernel (newer linux kernel version) on his system 
that has already
implemented the system call numbered 301. The customer now runs his program. 
Even if he compiles
it again he has the old header files, so that does not make a difference.

Now his program uses number 301 that refers to some other system call and so, 
we can see system
crash, or some very wrong behaviour. Making system calls more portable will 
ensure that atleast
the program gets an indication that something is wrong (error returned from the 
kernel that this
system call name is not matched). Or, if the vendor is actually successful in 
pushing its system
call to the mainline kernel, no one needs to worry about it. Everything will 
run happily.

However, people may say that, implementing custom system calls is not advocated 
by linux. And I
think it is not advocated precisely because of this reason that they are not 
portable.

Regards,
Amit



__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

>>On 1/1/07, Amit Choudhary <[EMAIL PROTECTED]> wrote:

>>+#define KFREE(x) \
>>+ do { \
>>+ kfree(x); \
>>+ x = NULL; \
>>+ } while(0)


>>NAK until you have actual callers for it. CONFIG_SLAB_DEBUG already
>>catches use after free and double-free so I don't see the point of
>>this.

Well, I am not proposing this as a debugging aid. The idea is about correct 
programming, atleast
from my view. Ideally, if you kfree(x), then you should set x to NULL. So, 
either programmers do
it themselves or a ready made macro do it for them.

In my opinion, the programmers may welcome the macro that does it for them.

There is another good part about it that results in better programming and 
shorter code.

Consider an array x[10]. I allocate memory for all of them and then kfree them 
- kfree(x[0]),
kfree(x[1]), etc. But I do not set these elements to NULL. So,

x[0] = _location_0_already_freed
x[1] = _location_1_already_freed

... and so on...

Now, consider that when I try to allocate memory again, memory allocation fails 
at x[2]. So, we
have now,

x[0] = _valid_location_0
x[1] = _valid_location_1
x[2] = NULL
x[3] = _location_3_already_freed

So, now to avoid error path leak I have to free all the allocated memory. For 
this I have to
remember where the allocation failed because I cannot do kfree(x[3]) because it 
will crash.

You can easily visualize that how KFREE would have helped. Since, I have 
already KFREE'D them
earlier, x[3] is guaranteed to be NULL and I can free the entire array 'x' 
wihtout worrying.

So, the code becomes simpler.

Now, consider that there are two more arrays like 'x' being used in the same 
function. So, now we
have 'x', 'y', 'z' all allocating memory in the same function. Memory 
allocation can fail at
anytime. So, not only do I have to remember the element location where the 
allocation failed but
also the array that contains that element.

So, to avoid error path leak, we will have something like this (assume same 
number of elements in
all arrays):

case z_nomem:
  free_from_0_to_i
  free_all_'y'
  free_all_'x'
  return;

case y_nomem:
  free_from_0_to_i
  free_all_'x'
  return;

case x_nomem:
  free_from_0_to_i
  return;

However, if the programmer would have used KFREE, then the error path leak code 
have been shorter
and easier.

case nomem:
  free_all_'z'
  free_all_'y'
  free_all_'x'
  return;

I hope that I have made my point but please let me know if I have missed out 
something or made
some assumption that is not correct.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[DISCUSS] Making system calls more portable.

[Amit Choudhary]

Hi,

I wanted to know if there is any inclination towards making system calls more 
portable. Please let
me know if this discussion has happened before.

Well, system calls today are not portable mainly because they are invoked using 
a number and it
may happen that a number 'N' may refer to systemcall_1() on one system/kernel 
and to
systemcall_2() on another system/kernel. This problem may surface if you 
compile your program
using headers from version_1 of the kernel, and then install another version of 
the kernel or a
custom kernel that has extended the system call table (on the same system). If 
we want to improve
the portability then we can avoid this approach or improve this approach. It 
may or may not be
complex to implement these.

1. Invoke a system call using its name. Pass its name to the kernel as an 
argument of syscall() or
some other function. Probably may make the invocation of the system call 
slower. If the name
doesn't match in the kernel then an error can be returned.

2. Create a /proc entry that will return the number of the system call given 
its name. This number
can then be used to invoke the system call.

These approaches will also remove the dependency from user space header file 
that contains the
mapping from the system call name to its number. I hope that I made some sense.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[DISCUSS] Making system calls more portable.

[Amit Choudhary]

Hi,

I wanted to know if there is any inclination towards making system calls more 
portable. Please let
me know if this discussion has happened before.

Well, system calls today are not portable mainly because they are invoked using 
a number and it
may happen that a number 'N' may refer to systemcall_1() on one system/kernel 
and to
systemcall_2() on another system/kernel. This problem may surface if you 
compile your program
using headers from version_1 of the kernel, and then install another version of 
the kernel or a
custom kernel that has extended the system call table (on the same system). If 
we want to improve
the portability then we can avoid this approach or improve this approach. It 
may or may not be
complex to implement these.

1. Invoke a system call using its name. Pass its name to the kernel as an 
argument of syscall() or
some other function. Probably may make the invocation of the system call 
slower. If the name
doesn't match in the kernel then an error can be returned.

2. Create a /proc entry that will return the number of the system call given 
its name. This number
can then be used to invoke the system call.

These approaches will also remove the dependency from user space header file 
that contains the
mapping from the system call name to its number. I hope that I made some sense.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

On 1/1/07, Amit Choudhary [EMAIL PROTECTED] wrote:

+#define KFREE(x) \
+ do { \
+ kfree(x); \
+ x = NULL; \
+ } while(0)


NAK until you have actual callers for it. CONFIG_SLAB_DEBUG already
catches use after free and double-free so I don't see the point of
this.

Well, I am not proposing this as a debugging aid. The idea is about correct 
programming, atleast
from my view. Ideally, if you kfree(x), then you should set x to NULL. So, 
either programmers do
it themselves or a ready made macro do it for them.

In my opinion, the programmers may welcome the macro that does it for them.

There is another good part about it that results in better programming and 
shorter code.

Consider an array x[10]. I allocate memory for all of them and then kfree them 
- kfree(x[0]),
kfree(x[1]), etc. But I do not set these elements to NULL. So,

x[0] = _location_0_already_freed
x[1] = _location_1_already_freed

... and so on...

Now, consider that when I try to allocate memory again, memory allocation fails 
at x[2]. So, we
have now,

x[0] = _valid_location_0
x[1] = _valid_location_1
x[2] = NULL
x[3] = _location_3_already_freed

So, now to avoid error path leak I have to free all the allocated memory. For 
this I have to
remember where the allocation failed because I cannot do kfree(x[3]) because it 
will crash.

You can easily visualize that how KFREE would have helped. Since, I have 
already KFREE'D them
earlier, x[3] is guaranteed to be NULL and I can free the entire array 'x' 
wihtout worrying.

So, the code becomes simpler.

Now, consider that there are two more arrays like 'x' being used in the same 
function. So, now we
have 'x', 'y', 'z' all allocating memory in the same function. Memory 
allocation can fail at
anytime. So, not only do I have to remember the element location where the 
allocation failed but
also the array that contains that element.

So, to avoid error path leak, we will have something like this (assume same 
number of elements in
all arrays):

case z_nomem:
  free_from_0_to_i
  free_all_'y'
  free_all_'x'
  return;

case y_nomem:
  free_from_0_to_i
  free_all_'x'
  return;

case x_nomem:
  free_from_0_to_i
  return;

However, if the programmer would have used KFREE, then the error path leak code 
have been shorter
and easier.

case nomem:
  free_all_'z'
  free_all_'y'
  free_all_'x'
  return;

I hope that I have made my point but please let me know if I have missed out 
something or made
some assumption that is not correct.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [DISCUSS] Making system calls more portable.

[Amit Choudhary]

--- Rene Herman [EMAIL PROTECTED] wrote:

If we're limited to Linux kernels, this seems to not be the case. Great care 
is taken in keeping
this userspace ABI stable -- new system calls are given new numbers. Old 
system calls may
disappear (after a long grace period) but even then I don't believe the number 
is ever recycled.

 If your discussion is not limited to Linux kernels, then sure, but being 
 portable at that (sub-libc) level is asking too much.
 

I will come to the main issue later but I just wanted to point out that we 
maintain information at
two separate places - mapping between the name and the number in user space and 
kernel space.
Shouldn't this duplication be removed.

Now, let's say a vendor has linux_kernel_version_1 that has 300 system calls. 
The vendor needs to
give some extra functionality to its customers and the way chosen is to 
implement new system call.
 The new system call number is 301. The customer gets this custom kernel and 
uses number 301.
Next, he downloads another kernel (newer linux kernel version) on his system 
that has already
implemented the system call numbered 301. The customer now runs his program. 
Even if he compiles
it again he has the old header files, so that does not make a difference.

Now his program uses number 301 that refers to some other system call and so, 
we can see system
crash, or some very wrong behaviour. Making system calls more portable will 
ensure that atleast
the program gets an indication that something is wrong (error returned from the 
kernel that this
system call name is not matched). Or, if the vendor is actually successful in 
pushing its system
call to the mainline kernel, no one needs to worry about it. Everything will 
run happily.

However, people may say that, implementing custom system calls is not advocated 
by linux. And I
think it is not advocated precisely because of this reason that they are not 
portable.

Regards,
Amit



__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Christoph Hellwig [EMAIL PROTECTED] wrote:

 On Sun, Jan 07, 2007 at 12:46:50AM -0800, Amit Choudhary wrote:
  Well, I am not proposing this as a debugging aid. The idea is about correct 
  programming,
 atleast
  from my view. Ideally, if you kfree(x), then you should set x to NULL. So, 
  either programmers
 do
  it themselves or a ready made macro do it for them.
 
 No, you should not.  I suspect that's the basic point you're missing.
 
 

Any strong reason why not? x has some value that does not make sense and can 
create only problems.
And as I explained, it can result in longer code too. So, why keep this value 
around. Why not
re-initialize it to NULL.

If x should not be re-initialized to NULL, then by the same logic, we should 
not even initialize
local variables. And all of us know that local variables should be initialized.

I would like to know a good reason as to why x should not be set to NULL.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov [EMAIL PROTECTED] wrote:

 On Sun, 2007-01-07 at 14:43 -0800, Amit Choudhary wrote:
  Any strong reason why not? x has some value that does not make sense and 
  can create only
 problems.
  And as I explained, it can result in longer code too. So, why keep this 
  value around. Why not
  re-initialize it to NULL.
 
 Because it looks really STRANGE(tm). Consider the following function,
 which is essentially what you're proposing in macro-ized form:
   void foobar(void)
   {
   void *ptr;
 
   ptr = kmalloc(...);
   // actual work here
   kfree(ptr);
   ptr = NULL;
   }

That's where KFREE(ptr) comes in so that the code doesn't look ugly and still 
the purpose is
achieved.

I still do not know of a single good reason as to why we should not do this.

And if all programmers did the right thing always then why do we have all the 
debugging options in
the first place.

 Reading code like that makes me say wtf?, simply because 'ptr' is not
 used thereafter,

Really? Then why do we have all the debugging options to catch re-use of the 
memory that has been
freed. So many debugging options has been implemented, so much effort has gone 
into them, partly
because programmers sometimes miss correct programming.

 so setting it to NULL is both pointless and confusing
 (it looks out-of-place, and therefore makes me wonder if there's
 something stupidly tricky going on).
 
 Also, arguably, your demonstration of why the lack of the proposed
 KFREE() macro results in longer code is invalid. Whereas you wrote:
   pointer *arr_x[size_x];
   pointer *arr_y[size_y];
   pointer *arr_z[size_z];
 That really should have been:
   pointer *arr[size_x + size_y + size_z];
 or:
   pointer **arr[3] = { arr_x, arr_y, arr_z };
 In which case, the you only need one path in the function to handle
 allocation failures, rather than the three that you were arguing for.
 

I do not know what you are talking about here. You are saying that a function 
does not need three
different arrays with different names. How can you say that? How do you know 
what is the
requirement?

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov [EMAIL PROTECTED] wrote:

 
 struct type1 {
   /* something */
 };
 
 struct type2 {
   /* something */
 };
 
 #define COUNT 10
 
 void function1(struct type1 **a1, struct type2 **a2, unsigned int sz);
 
 void function2(void)
 {
   struct type1 *arr1[COUNT];
   struct type2 *arr2[COUNT];
   int i;
 
   /* init */
   for (i = 0; i  COUNT; i++) {
   arr1[i] = kmalloc(sizeof(struct type1), ...);
   if (!arr1[i])
   goto free_1;
   }
   for (i = 0; i  COUNT; i++) {
   arr2[i] = kmalloc(sizeof(struct type2), ...);
   if (!arr2[i])
   goto free_2;
   }
 
   /* work */
   function1(arr1, arr2, COUNT);
 
   /* fini */
   i = COUNT;
 free_2:
   for (i--; i = 0; i--) {
   kfree(arr2[i]);
   }
   i = COUNT;
 free_1:
   for (i--; i = 0; i--) {
   kfree(arr1[i]);
   }
 }
 
 In most cases, though, the above code design would be brain-damaged from
 the start: unless the sizes involved are prohibitively large, the
 function should be allocating all the memory in a single pass.
 
 So, where's the demonstrated need for KFREE()?

I have already explained it earlier. I will try again. You will not need 
free_2: and free_1: with
KFREE(). You will only need one free: with KFREE.

Also, let's say that count is different for each array? Then how do you propose 
that memory be
allocated in one pass?

In most cases, though, the above code design would be brain-damaged from
the start: unless the sizes involved are prohibitively large, the
function should be allocating all the memory in a single pass.

Well, only if everything would be fine and correct, we would not be needing 
anything. If you think
this kind of code is brain-damaged then the linux kernel has couple of these.

I have scanned the whole kernel to check whether people are checking for return 
values of kmalloc,
I found that at many places they don't and have sent patches for them. Now, 
this too is brain
damaged code. And during the scan I saw examples of what I described earlier.

KFREE() can fix some of those cases.

Consider this as the proof of what I explained earlier. This fucntion was 
broken but I fixed it
and then realized why KFREE() is needed. 2 kmallocs and 1 usb_alloc_urb(). 
Well, I can only give
examples why KFREE() is needed. If you do not agree, I cannot force you to 
agree with me. And if
you really do not want to agree then even my examples will fail. Also, if you 
think that people
are not doing KFREE() kind of stuff then you should scan the kernel and you 
will see it for
yourself.


Below are some examples where people are doing KFREE() kind of stuff:

--
arch/ppc/kernel/smp-tbsync.c:   kfree( tbsync );
arch/ppc/kernel/smp-tbsync.c-   tbsync = NULL;
--
arch/ppc/8260_io/fcc_enet.c:
dev_kfree_skb(fep-tx_skbuff[i]);
arch/ppc/8260_io/fcc_enet.c-fep-tx_skbuff[i] = NULL;
--
arch/ppc/8xx_io/cs4218_tdm.c:   kfree (sound_buffers);
arch/ppc/8xx_io/cs4218_tdm.c-   sound_buffers = 0;
--
arch/ia64/kernel/topology.c:kfree(all_cpu_cache_info[cpu].cache_leaves);
arch/ia64/kernel/topology.c-all_cpu_cache_info[cpu].cache_leaves = NULL;
--
arch/ia64/kernel/acpi.c:kfree(buffer.pointer);
arch/ia64/kernel/acpi.c-buffer.pointer = NULL;
--
arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c:
kfree(acpi_perf_data[j]);
arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c-
acpi_perf_data[j] = NULL;
--

There are many more and you can scan the kernel yourself.

Below is where memory is allocated in different arrays with different counts:

static int stv680_start_stream (struct usb_stv *stv680)
{
struct urb *urb;
int err = 0, i;

stv680-streaming = 1;

/* Do some memory allocation */
for (i = 0; i  STV680_NUMFRAMES; i++) {
stv680-frame[i].data = stv680-fbuf + i * stv680-maxframesize;
stv680-frame[i].curpix = 0;
}
/* packet size = 4096  */
for (i = 0; i  STV680_NUMSBUF; i++) {
stv680-sbuf[i].data = kmalloc (stv680-rawbufsize, GFP_KERNEL);
if (stv680-sbuf[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw data buffer 
%i, i);
goto nomem_err;
}
}

stv680-scratch_next = 0;
stv680-scratch_use = 0;
stv680-scratch_overflow = 0;
for (i = 0; i  STV680_NUMSCRATCH; i++) {
stv680-scratch[i].data = kmalloc (stv680-rawbufsize, 
GFP_KERNEL);
if (stv680-scratch[i].data == NULL) {
PDEBUG (0, STV(e): Could not kmalloc raw scratch 
buffer %i, i);
goto nomem_err;
}
stv680-scratch[i].state = BUFFER_UNUSED;

Re: [PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

--- Vadim Lobanov [EMAIL PROTECTED] wrote:

 On Sun, 2007-01-07 at 20:09 -0800, Amit Choudhary wrote:
  I have already explained it earlier. I will try again. You will not need 
  free_2: and free_1:
 with
  KFREE(). You will only need one free: with KFREE.
 

I do not want to write this but I think that you are arguing just for the heck 
of it. Please be
sane.

-Amit

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RESEND] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

Description: new KFREE() macro to set the variable NULL after freeing it.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1ef822e..28da74c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -75,6 +75,12 @@ void *__kzalloc(size_t, gfp_t);
 void kfree(const void *);
 unsigned int ksize(const void *);
 
+#define KFREE(x)   \
+   do {\
+   kfree(x);   \
+   x = NULL;   \
+   } while(0)
+
 /**
  * kcalloc - allocate memory for an array. The memory is set to zero.
  * @n: number of elements.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RESEND] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

Description: new KFREE() macro to set the variable NULL after freeing it.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1ef822e..28da74c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -75,6 +75,12 @@ void *__kzalloc(size_t, gfp_t);
 void kfree(const void *);
 unsigned int ksize(const void *);
 
+#define KFREE(x)   \
+   do {\
+   kfree(x);   \
+   x = NULL;   \
+   } while(0)
+
 /**
  * kcalloc - allocate memory for an array. The memory is set to zero.
  * @n: number of elements.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [DISCUSS] Make the variable NULL after freeing it.

[Amit Choudhary]

--- Ingo Oeser <[EMAIL PROTECTED]> wrote:

> On Sunday, 31. December 2006 14:38, Bernd Petrovitsch wrote:
> > That depends on the decision/definition if (so called) "double free" is
> > an error or not (and "free(NULL)" must work in POSIX-compliant
> > environments).
> 
> A double free of non-NULL is certainly an error.
> So the idea of setting it to NULL is ok, since then you can
> kfree the variable over and over again without any harm.
> 
> It is just complicated to do this side effect free.
> 
> Maybe one should check for builtin-constant and take the address,
> if this is not an builtin-constant.
> 
> sth, like this
> 
> #define kfree_nullify(x) do { \
>   if (__builtin_constant_p(x)) { \
>   kfree(x); \
>   } else { \
>   typeof(x) *__addr_x =  \
>   kfree(*__addr_x); \
>   *__addr_x = NULL; \
>   } \
> } while (0)
> 
> Regards
> 
> Ingo Oeser
> 

This is a nice approach but what if someone does kfree_nullify(x+20).

I decided to keep it simple. If someone is calling kfree_nullify() with 
anything other than a
simple variable, then they should call kfree().  But definitely an approach 
that takes care of all
situations is the best but I cannot think of a macro that can handle all 
situations. The simple
macro that I sent earlier will catch all the other usage at compile time. 
Please let me know if I
have missed something.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

Description: new KFREE() macro to set the variable NULL after freeing it.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1ef822e..28da74c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -75,6 +75,12 @@ void *__kzalloc(size_t, gfp_t);
 void kfree(const void *);
 unsigned int ksize(const void *);
 
+#define KFREE(x)   \
+   do {\
+   kfree(x);   \
+   x = NULL;   \
+   } while(0)
+
 /**
  * kcalloc - allocate memory for an array. The memory is set to zero.
  * @n: number of elements.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] drivers/char/agp/sgi-agp.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function agp_sgi_init(), in 
file drivers/char/agp/sgi-agp.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c
index d73be4c..5897e6c 100644
--- a/drivers/char/agp/sgi-agp.c
+++ b/drivers/char/agp/sgi-agp.c
@@ -285,6 +285,8 @@ static int __devinit agp_sgi_init(void)
(struct agp_bridge_data **)kmalloc(tioca_gart_found *
   sizeof(struct agp_bridge_data *),
   GFP_KERNEL);
+   if (!sgi_tioca_agp_bridges)
+   return -ENOMEM;
 
j = 0;
list_for_each_entry(info, _list, ca_list) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] drivers/char/agp/sgi-agp.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function agp_sgi_init(), in 
file drivers/char/agp/sgi-agp.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c
index d73be4c..5897e6c 100644
--- a/drivers/char/agp/sgi-agp.c
+++ b/drivers/char/agp/sgi-agp.c
@@ -285,6 +285,8 @@ static int __devinit agp_sgi_init(void)
(struct agp_bridge_data **)kmalloc(tioca_gart_found *
   sizeof(struct agp_bridge_data *),
   GFP_KERNEL);
+   if (!sgi_tioca_agp_bridges)
+   return -ENOMEM;
 
j = 0;
list_for_each_entry(info, tioca_list, ca_list) {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] include/linux/slab.h: new KFREE() macro.

[Amit Choudhary]

Description: new KFREE() macro to set the variable NULL after freeing it.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1ef822e..28da74c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -75,6 +75,12 @@ void *__kzalloc(size_t, gfp_t);
 void kfree(const void *);
 unsigned int ksize(const void *);
 
+#define KFREE(x)   \
+   do {\
+   kfree(x);   \
+   x = NULL;   \
+   } while(0)
+
 /**
  * kcalloc - allocate memory for an array. The memory is set to zero.
  * @n: number of elements.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [DISCUSS] Make the variable NULL after freeing it.

[Amit Choudhary]

--- Ingo Oeser [EMAIL PROTECTED] wrote:

 On Sunday, 31. December 2006 14:38, Bernd Petrovitsch wrote:
  That depends on the decision/definition if (so called) double free is
  an error or not (and free(NULL) must work in POSIX-compliant
  environments).
 
 A double free of non-NULL is certainly an error.
 So the idea of setting it to NULL is ok, since then you can
 kfree the variable over and over again without any harm.
 
 It is just complicated to do this side effect free.
 
 Maybe one should check for builtin-constant and take the address,
 if this is not an builtin-constant.
 
 sth, like this
 
 #define kfree_nullify(x) do { \
   if (__builtin_constant_p(x)) { \
   kfree(x); \
   } else { \
   typeof(x) *__addr_x = x; \
   kfree(*__addr_x); \
   *__addr_x = NULL; \
   } \
 } while (0)
 
 Regards
 
 Ingo Oeser
 

This is a nice approach but what if someone does kfree_nullify(x+20).

I decided to keep it simple. If someone is calling kfree_nullify() with 
anything other than a
simple variable, then they should call kfree().  But definitely an approach 
that takes care of all
situations is the best but I cannot think of a macro that can handle all 
situations. The simple
macro that I sent earlier will catch all the other usage at compile time. 
Please let me know if I
have missed something.

-Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] [BUGFIX] drivers/atm/firestream.c: Fix infinite recursion when alignment passed is 0.

[Amit Choudhary]

Description: Fix infinite recursion when alignment passed is 0 in function 
aligned_kmalloc(), in file drivers/atm/firestream.c. Also, a negative value for 
alignment does not make sense. Check for negative value too.
The function prototype is:
static void __devinit *aligned_kmalloc (int size, gfp_t flags, 
int alignment).


Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index 9c67df5..2ba6b2e 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -1385,7 +1385,7 @@ static void __devinit *aligned_kmalloc (
 
if (alignment <= 0x10) {
t = kmalloc (size, flags);
-   if ((unsigned long)t & (alignment-1)) {
+   if ((unsigned long)t & ((alignment <= 0) ? 0 : (alignment-1))) {
printk ("Kmalloc doesn't align things correctly! %p\n", 
t);
kfree (t);
return aligned_kmalloc (size, flags, alignment * 4);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] [BUGFIX] drivers/atm/firestream.c: Fix infinite recursion when alignment passed is 0.

[Amit Choudhary]

Description: Fix infinite recursion when alignment passed is 0 in function 
aligned_kmalloc(), in file drivers/atm/firestream.c. Also, a negative value for 
alignment does not make sense. Check for negative value too.
The function prototype is:
static void __devinit *aligned_kmalloc (int size, gfp_t flags, 
int alignment).


Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index 9c67df5..2ba6b2e 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -1385,7 +1385,7 @@ static void __devinit *aligned_kmalloc (
 
if (alignment = 0x10) {
t = kmalloc (size, flags);
-   if ((unsigned long)t  (alignment-1)) {
+   if ((unsigned long)t  ((alignment = 0) ? 0 : (alignment-1))) {
printk (Kmalloc doesn't align things correctly! %p\n, 
t);
kfree (t);
return aligned_kmalloc (size, flags, alignment * 4);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] fs/jffs2/scan.c: Fix error-path leak

[Amit Choudhary]

Description: Fix error-path leak in function jffs2_scan_medium(), in file 
fs/jffs2/scan.c

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index e241346..cd9ed6e 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -130,6 +130,8 @@ #endif
if (jffs2_sum_active()) {
s = kmalloc(sizeof(struct jffs2_summary), GFP_KERNEL);
if (!s) {
+   free(flashbuf);
+   flashbuf = NULL;
JFFS2_WARNING("Can't allocate memory for summary\n");
return -ENOMEM;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.20-rc2] fs/jffs2/scan.c: Fix error-path leak

[Amit Choudhary]

Description: Fix error-path leak in function jffs2_scan_medium(), in file 
fs/jffs2/scan.c

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index e241346..cd9ed6e 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -130,6 +130,8 @@ #endif
if (jffs2_sum_active()) {
s = kmalloc(sizeof(struct jffs2_summary), GFP_KERNEL);
if (!s) {
+   free(flashbuf);
+   flashbuf = NULL;
JFFS2_WARNING(Can't allocate memory for summary\n);
return -ENOMEM;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [DISCUSS] Make the variable NULL after freeing it.

[Amit Choudhary]

Hi,

Was just wondering if the _var_ in kfree(_var_) could be set to NULL after its 
freed. It may solve
the problem of accessing some freed memory as the kernel will crash since _var_ 
was set to NULL.

Does this make sense? If yes, then how about renaming kfree to something else 
and providing a
kfree macro that would do the following:

#define kfree(x) do { \
  new_kfree(x); \
  x = NULL; \
} while(0)

There might be other better ways too.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [DISCUSS] Make the variable NULL after freeing it.

[Amit Choudhary]

Hi,

Was just wondering if the _var_ in kfree(_var_) could be set to NULL after its 
freed. It may solve
the problem of accessing some freed memory as the kernel will crash since _var_ 
was set to NULL.

Does this make sense? If yes, then how about renaming kfree to something else 
and providing a
kfree macro that would do the following:

#define kfree(x) do { \
  new_kfree(x); \
  x = NULL; \
} while(0)

There might be other better ways too.

Regards,
Amit


__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] drivers/char/vt.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function con_init(), in 
file drivers/char/vt.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index 87587b4..6aa08cb 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -2640,6 +2640,15 @@ static int __init con_init(void)
 */
for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
vc_cons[currcons].d = vc = alloc_bootmem(sizeof(struct 
vc_data));
+   if (!vc_cons[currcons].d) {
+   for (--currcons; currcons >= 0; currcons--) {
+   kfree(vc_cons[currcons].d);
+   vc_cons[currcons].d = NULL;
+   }
+   release_console_sem();
+   return -ENOMEM;
+   }
+
visual_init(vc, currcons, 1);
vc->vc_screenbuf = (unsigned short 
*)alloc_bootmem(vc->vc_screenbuf_size);
vc->vc_kmalloced = 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] net/wanrouter/wanmain.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function dbg_kmalloc(), in 
file net/wanrouter/wanmain.c.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/net/wanrouter/wanmain.c b/net/wanrouter/wanmain.c
index 316211d..263450c 100644
--- a/net/wanrouter/wanmain.c
+++ b/net/wanrouter/wanmain.c
@@ -67,6 +67,8 @@ static void * dbg_kmalloc(unsigned int s
int i = 0;
void * v = kmalloc(size+sizeof(unsigned int)+2*KMEM_SAFETYZONE*8,prio);
char * c1 = v;
+   if (!v)
+   return NULL;
c1 += sizeof(unsigned int);
*((unsigned int *)v) = size;
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] drivers/media/video/cpia2/cpia2_usb.c: Free previously allocated memory (in array elements) if kmalloc() returns NULL.

[Amit Choudhary]

Description: Free previously allocated memory (in array elements) if kmalloc() 
returns NULL, in function submit_urbs(), in file 
drivers/media/video/cpia2/cpia2_usb.c. If the system is low on memory, then 
previously allocated memory in the same array should be freed up to help the 
system recover.

Signed-off-by: Amit Choudhary <[EMAIL PROTECTED]>

diff --git a/drivers/media/video/cpia2/cpia2_usb.c 
b/drivers/media/video/cpia2/cpia2_usb.c
index 28dc6a1..c938638 100644
--- a/drivers/media/video/cpia2/cpia2_usb.c
+++ b/drivers/media/video/cpia2/cpia2_usb.c
@@ -640,6 +640,10 @@ static int submit_urbs(struct camera_dat
cam->sbuf[i].data =
kmalloc(FRAMES_PER_DESC * FRAME_SIZE_PER_DESC, GFP_KERNEL);
if (!cam->sbuf[i].data) {
+   for (--i; i >= 0; i--) {
+   kfree(cam->sbuf[i].data);
+   cam->sbuf[i].data = NULL;
+   }
return -ENOMEM;
}
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] drivers/media/video/cpia2/cpia2_usb.c: Free previously allocated memory (in array elements) if kmalloc() returns NULL.

[Amit Choudhary]

Description: Free previously allocated memory (in array elements) if kmalloc() 
returns NULL, in function submit_urbs(), in file 
drivers/media/video/cpia2/cpia2_usb.c. If the system is low on memory, then 
previously allocated memory in the same array should be freed up to help the 
system recover.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/media/video/cpia2/cpia2_usb.c 
b/drivers/media/video/cpia2/cpia2_usb.c
index 28dc6a1..c938638 100644
--- a/drivers/media/video/cpia2/cpia2_usb.c
+++ b/drivers/media/video/cpia2/cpia2_usb.c
@@ -640,6 +640,10 @@ static int submit_urbs(struct camera_dat
cam-sbuf[i].data =
kmalloc(FRAMES_PER_DESC * FRAME_SIZE_PER_DESC, GFP_KERNEL);
if (!cam-sbuf[i].data) {
+   for (--i; i = 0; i--) {
+   kfree(cam-sbuf[i].data);
+   cam-sbuf[i].data = NULL;
+   }
return -ENOMEM;
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] net/wanrouter/wanmain.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function dbg_kmalloc(), in 
file net/wanrouter/wanmain.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/net/wanrouter/wanmain.c b/net/wanrouter/wanmain.c
index 316211d..263450c 100644
--- a/net/wanrouter/wanmain.c
+++ b/net/wanrouter/wanmain.c
@@ -67,6 +67,8 @@ static void * dbg_kmalloc(unsigned int s
int i = 0;
void * v = kmalloc(size+sizeof(unsigned int)+2*KMEM_SAFETYZONE*8,prio);
char * c1 = v;
+   if (!v)
+   return NULL;
c1 += sizeof(unsigned int);
*((unsigned int *)v) = size;
 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2.6.19] drivers/char/vt.c: check kmalloc() return value.

[Amit Choudhary]

Description: Check the return value of kmalloc() in function con_init(), in 
file drivers/char/vt.c.

Signed-off-by: Amit Choudhary [EMAIL PROTECTED]

diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index 87587b4..6aa08cb 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -2640,6 +2640,15 @@ static int __init con_init(void)
 */
for (currcons = 0; currcons  MIN_NR_CONSOLES; currcons++) {
vc_cons[currcons].d = vc = alloc_bootmem(sizeof(struct 
vc_data));
+   if (!vc_cons[currcons].d) {
+   for (--currcons; currcons = 0; currcons--) {
+   kfree(vc_cons[currcons].d);
+   vc_cons[currcons].d = NULL;
+   }
+   release_console_sem();
+   return -ENOMEM;
+   }
+
visual_init(vc, currcons, 1);
vc-vc_screenbuf = (unsigned short 
*)alloc_bootmem(vc-vc_screenbuf_size);
vc-vc_kmalloced = 0;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/