Re: [PATCH] staging: ft1000: fix error path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux