On Sun, Mar 13, 2011 at 09:58:46PM +0300, Alexander Beregalov wrote:
> Free resources before exit.
>
> Signed-off-by: Alexander Beregalov <a.beregalov@xxxxxxxxx>
> ---
> diff --git a/drivers/staging/bcm/CmHost.c b/drivers/staging/bcm/CmHost.c
> index 017b471..0587f42 100644
> --- a/drivers/staging/bcm/CmHost.c
> +++ b/drivers/staging/bcm/CmHost.c
> @@ -1671,6 +1671,7 @@ ULONG StoreCmControlResponseMessage(PMINI_ADAPTER Adapter,PVOID pvBuffer,UINT *p
> stLocalSFDeleteRequest *pstDeletionRequest;
> UINT uiSearchRuleIndex;
> ULONG ulSFID;
> + ULONG ret;
Just initialize ret here.
> /* this can't possibly be right */
Hahah. Probably this comment is correct. :P
> pstAddIndication->psfActiveSet = (stServiceFlowParamSI *)ntohl((ULONG)pstAddIndication->psfActiveSet);
>
> (*puBufferLength) = sizeof(stLocalSFAddIndication);
> *(stLocalSFAddIndication *)pvBuffer = *pstAddIndication;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If we ever get to this line then we'll oops. It's trying to dereference
the first elements of pstAddIndication which are unsigned chars and not
pointers. Why doesn't GCC warn about this?
> +
> + ret = 1;
> +
> +exit:
> kfree(pstAddIndication);
And then we free pstAddIndication which we were trying to keep. So this
code is really broken, and it needs to be rewritten to fix the crash.
At the very least we should check again whether we should free the
struct on the success path.
Maybe resubmit the patch to the other file without this one and let the
bcm devs looking into this function.
regards,
dan carpenter
> - return 1;
> + return ret;
> }
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
