Dynamic memory management in CryptAcquireContext

[Michael Jung]

Alexandre,

I've noticed that you applied a modified version of my patch, which tries to 
fix a potential double free of the "temp" variable in dlls/advapi32/crypt.c's 
CryptAcquireContextA function. However, I think that the modified version 
does not fix the problem. Here is a code snippet from the current cvs 
versionc of crypt.c:

  CRYPT_Free(temp);
  ...
  if (pProv->pFuncs->pCPAcquireContext(&pProv->hPrivate, (CHAR*)pszContainer, 
dwFlags, pProv->pVTable))
  {
    ...
    return TRUE;
  }
  /* FALLTHROUGH TO ERROR IF FALSE - CSP internal error! */
error:
  ...
  CRYPT_Free(temp);
  ...
  return FALSE;

If the CSP's CPAcquireContext function fails, which is quite common (e.g. 
because the user specified key container does not exist), the "temp" variable 
is still free'd twice.

I'm aware that my original patch was pretty ugly, due to a lot of redundancy, 
but I think this is due to fact that the CryptAcquireContext function seems 
not to be very well structured. I've submitted a patch two weeks ago, which 
basically was a reimplementation of CryptAcquireContext, but which was not 
committed (http://www.winehq.org/hypermail/wine-patches/2004/07/0213.html). 
Rob Shearman commented on this patch and felt that it is overkill to add both 
a helper function and a new struct type. Is this the reason the patch was not 
applied?

I would be glad to rework the patch, if you give me some hints about what you 
did not like. However, I would prefer to wait until James Hawkins got his 
a->w cross call cleanup patch committed, in order not to break his work. 

Greetings,
Michael

-- 
Michael Jung, MSc                               Be liberal in what you accept,
Integrated Circuits and Systems Lab         and conservative in what you send. 
University of Technology, Darmstadt       -- Jonathan Postel, Internet Pioneer
phone: +49(6151)16-6692 / email: mjung at iss.tu-darmstadt.de