hi Marek, IMHO the alloc sequencs is building a linked list of allocated buffer. And if kfree() does no magiclly follow the list this is not working. re, wh Belisko Marek schrieb: > On Sun, Sep 26, 2010 at 3:11 PM, Dan Carpenter <error27@xxxxxxxxx> wrote: >> On Sun, Sep 26, 2010 at 12:59:55PM +0400, Vasiliy Kulikov wrote: >>> +err_free: >>> + for (i--; i>=0; i--) { >>> + kfree(pdpram_blk->pbuffer); >>> + kfree(pdpram_blk); >>> + } >> This is wrong. I don't have linux-next so I can't see the context, why >> are we looping here? The second iteration through the loop will cause a >> NULL dereference. > Some lines upper there is allocation of structure and it's internal > buffer in loop: > for (i=0; i<NUM_OF_FREE_BUFFERS; i++) { > // Get memory for DPRAM_DATA link list > pdpram_blk = kmalloc ( sizeof(DPRAM_BLK), GFP_KERNEL ); > // Get a block of memory to store command data > pdpram_blk->pbuffer = kmalloc ( MAX_CMD_SQSIZE, GFP_KERNEL ); > // link provisioning data > list_add_tail (&pdpram_blk->list, &freercvpool); > } > > Free loop is correct in my opinion but kfree should be extended by checking > of NULL pointer because allocation of pdpram_blk could fail and we free also > pdpram_blk->pbuffer. >> Also there should be spaces before and after the ">=". >> >> regards, >> dan carpenter >> >>> + return STATUS_FAILURE; >>> } >>> >> > > marek > -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html