>> Michal Nazarewicz [m.nazarewicz@xxxxxxxxxxx]
>>> In to places in fsg_common_init() an unconditional call to kfree()
>>> on common was performed in error recovery which is not a valid
>On Wed, 12 May 2010 10:29:02 +0200, Viral Mehta <Viral.Mehta@xxxxxxxxxxxxxxx> wrote:
>> If I am not wrong, common can not be NULL since it was already checked above.
>>
>> 2685 if (!common) {
>> 2686 common = kzalloc(sizeof *common, GFP_KERNEL);
>> 2687 if (!common)
>> 2688 return ERR_PTR(-ENOMEM);
>> 2689 common->free_storage_on_release = 1;
>That is correct but it is not the issue.
>fsg_common_init() as a first argument takes a pointer to a fsg_common
>structure which, if not NULL, is reused and it is then assumed that
>caller is responsible far maintaining allocation and deallocation of
>this structure.
>The idea is that one can do:
>static struct fsg_common fsg_common;
>/* ... */
>fsg_common_init(&fsg_common, cdev, fsg_config);
>or allocate fsg_common structure as a part of a larger structure. In such
>cases kfree() cannot be called on the object. Which case we are dealing
>with is indicated by the common->free_storage_on_release flag.
>fsg_common_release() consults it and either calls or does not call krfee().
Oh, makes sense. All instances has fsg_common_init(0, ...) and so i just missed...
But, still the central idea was, why should we go to whole error_release path which really does removing device file and closing luns and etc.
However, it will not make any difference since curlun->nluns will be zero and so there will be no loop in fsg_common_releas() function.
Apart from that, IMHO,
2894 kfree(common->luns);
should crash in case if your error path is followed.
common->luns is allocd
2712 /* Create the LUNs, open their backing files, and register the
2713 * LUN devices in sysfs. */
2714 curlun = kzalloc(nluns * sizeof *curlun, GFP_KERNEL);
2715 if (!curlun) {
2716 kfree(common);
2717 return ERR_PTR(-ENOMEM);
2718 }
Thanks,
Viral
______________________________________________________________________
This Email may contain confidential or privileged information for the intended recipient (s) If you are not the intended recipient, please do not use or disseminate the information, notify the sender and delete it from your system.
______________________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
