[Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Timmy Yee]

Hi,

In drivers/scsi/aha1542.c, kmalloc() is called with no memtype (i.e.
without some flag like GFP_KERNEL). The following patch will fix that.

--- linux-2.6.1/drivers/scsi/aha1542.c	2003-08-09 18:19:18.000000000 -0700
+++ linux-2.6.1-mpi/drivers/scsi/aha1542.c	2004-01-18 19:20:38.000000000 -0800
@@ -704,7 +704,7 @@
 #endif
 		int i;
 		ccb[mbo].op = 2;	/* SCSI Initiator Command  w/scatter-gather */
-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
 		cptr = (struct chain *) SCpnt->host_scribble;
 		if (cptr = NULL)
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Daniele Bellucci]

|-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
|+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| 		cptr = (struct chain *) SCpnt->host_scribble;

I didn't look to much at your patch but 
because you are at it can you please audit that kmalloc too?

tnx


-- 

Daniele.




"I could have made money this way, and perhaps amused myself writing code. 
But I knew that at the end of my career, I would look back on years of 
building walls to divide people, and feel I had spent my life making the 
world a worse place."                               
                                                          Richard Stallman

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Timmy Yee]

On Mon, Jan 19, 2004 at 06:00:41PM +0800, Eugene Teo wrote:

> What do you mean called with no memtype?

GFP_DMA is a helper flag that tells the kernel that the memory should be
suitable for DMA purposes. But it doesn't necessarily say what type of
memory it should be allocating (i.e. should the memory be allocated on
behalf of the user? should the allocation be done atomically? etc).
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Timmy Yee]

On Mon, Jan 19, 2004 at 01:37:15PM +0100, Daniele Bellucci wrote:
> 
> |-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
> |+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
> | 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
> | 		cptr = (struct chain *) SCpnt->host_scribble;
> 
> I didn't look to much at your patch but 
> because you are at it can you please audit that kmalloc too?

The address returned by kmalloc() seems to be checked three lines later.
Though it calls panic() if the address is NULL. I'm not sure if that's the
correct way to fail on an allocation.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Randy.Dunlap]

On Tue, 20 Jan 2004 07:33:10 -0800 Timmy Yee <shoujun@masterofpi.org> wrote:

| On Mon, Jan 19, 2004 at 01:37:15PM +0100, Daniele Bellucci wrote:
| > 
| > |-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
| > |+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| > | 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| > | 		cptr = (struct chain *) SCpnt->host_scribble;
| > 
| > I didn't look to much at your patch but 
| > because you are at it can you please audit that kmalloc too?
| 
| The address returned by kmalloc() seems to be checked three lines later.
| Though it calls panic() if the address is NULL. I'm not sure if that's the
| correct way to fail on an allocation.

Preferably not.

--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] fix kmalloc() in aha1542.c

[Randy.Dunlap]

On Tue, 20 Jan 2004 14:55:55 -0800 "Randy.Dunlap" <rddunlap@osdl.org> wrote:

| On Tue, 20 Jan 2004 07:33:10 -0800 Timmy Yee <shoujun@masterofpi.org> wrote:
| 
| | On Mon, Jan 19, 2004 at 01:37:15PM +0100, Daniele Bellucci wrote:
| | > 
| | > |-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
| | > |+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| | > | 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| | > | 		cptr = (struct chain *) SCpnt->host_scribble;
| | > 
| | > I didn't look to much at your patch but 
| | > because you are at it can you please audit that kmalloc too?
| | 
| | The address returned by kmalloc() seems to be checked three lines later.
| | Though it calls panic() if the address is NULL. I'm not sure if that's the
| | correct way to fail on an allocation.
| 
| Preferably not.

Better answer:
It should just return a non-0 value from the queuecommand()
function in that case, so that the SCSI mid-layer will retry
the command.

Timmy, care to send that patch?

Thanks,
--
~Randy
kernel-janitors project:  http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors