Bug in MapAndLoad function

[Adam Connell]

Greetings,

While investigating why a game (Kasparov's Chessmate) didn't work,
I think I found a bug in the implementation of imagehlp.dll in the
the MapAndLoad function located in dlls/imagehlp/access.c lines
165-167. The function looks like it tries to open an uninitialized
string szFileName instead of the passed file name "pszImageName".

I think there has been a bug filed already (Bug 6099 Alien Shooter
needs imagehlp.dll override) and I think this explains it.

Wine version 0.9.25

/***********************************************************************
 *		MapAndLoad (IMAGEHLP.@)
 */
BOOL WINAPI MapAndLoad(
  LPSTR pszImageName, LPSTR pszDllPath, PLOADED_IMAGE pLoadedImage,
  BOOL bDotDll, BOOL bReadOnly)
{
  CHAR szFileName[MAX_PATH];
  HANDLE hFile = NULL;
  HANDLE hFileMapping = NULL;
  HMODULE hModule = NULL;
  PIMAGE_NT_HEADERS pNtHeader = NULL;

  TRACE("(%s, %s, %p, %d, %d)\n", pszImageName, pszDllPath, 
pLoadedImage,
                                    bDotDll, bReadOnly);
  
  /* PathCombine(&szFileName, pszDllPath, pszImageName); */
  /* PathRenameExtension(&szFileName, bDotDll?:"dll":"exe"); */

  /* FIXME: Check if the file already loaded (use 
IMAGEHLP_pFirstLoadedImage) */
  if(!(hFile = CreateFileA(
    szFileName, GENERIC_READ, 1, /* FIXME: FILE_SHARE_READ not defined 
*/
    NULL, OPEN_EXISTING, 0, NULL)))
    {
      SetLastError(ERROR_FILE_NOT_FOUND);
      goto Error;
    }

Cheers,
Adam

[Aaron Slunt]

Adam Connell wrote:
> Greetings,
>
> While investigating why a game (Kasparov's Chessmate) didn't work,
> I think I found a bug in the implementation of imagehlp.dll in the
> the MapAndLoad function located in dlls/imagehlp/access.c lines
> 165-167. The function looks like it tries to open an uninitialized
> string szFileName instead of the passed file name "pszImageName".
>
> I think there has been a bug filed already (Bug 6099 Alien Shooter
> needs imagehlp.dll override) and I think this explains it.
>
> Wine version 0.9.25
>
> /***********************************************************************
>  *		MapAndLoad (IMAGEHLP.@)
>  */
> BOOL WINAPI MapAndLoad(
>   LPSTR pszImageName, LPSTR pszDllPath, PLOADED_IMAGE pLoadedImage,
>   BOOL bDotDll, BOOL bReadOnly)
> {
>   CHAR szFileName[MAX_PATH];
>   HANDLE hFile = NULL;
>   HANDLE hFileMapping = NULL;
>   HMODULE hModule = NULL;
>   PIMAGE_NT_HEADERS pNtHeader = NULL;
>
> TRACE("(%s, %s, %p, %d, %d)\n", pszImageName, pszDllPath, 
> pLoadedImage,
>                                     bDotDll, bReadOnly);
>
> /* PathCombine(&szFileName, pszDllPath, pszImageName); */
>   /* PathRenameExtension(&szFileName, bDotDll?:"dll":"exe"); */
>
> /* FIXME: Check if the file already loaded (use 
> IMAGEHLP_pFirstLoadedImage) */
>   if(!(hFile = CreateFileA(
>     szFileName, GENERIC_READ, 1, /* FIXME: FILE_SHARE_READ not defined 
> */
>     NULL, OPEN_EXISTING, 0, NULL)))
>     {
>       SetLastError(ERROR_FILE_NOT_FOUND);
>       goto Error;
>     }
>
> Cheers,
> Adam

Please file a bug in bugzilla regarding this. wine-devel is not the 
place to file bug reports, much more can get done in bugzilla.

http://bugs.winehq.org

Thank you.

[Aaron Slunt]

Aaron Slunt wrote:
> Adam Connell wrote:
> > Greetings,
> >
> > While investigating why a game (Kasparov's Chessmate) didn't work,
> > I think I found a bug in the implementation of imagehlp.dll in the
> > the MapAndLoad function located in dlls/imagehlp/access.c lines
> > 165-167. The function looks like it tries to open an uninitialized
> > string szFileName instead of the passed file name "pszImageName".
> >
> > I think there has been a bug filed already (Bug 6099 Alien Shooter
> > needs imagehlp.dll override) and I think this explains it.
> >
> > Wine version 0.9.25
> >
> > /***********************************************************************
> >  *        MapAndLoad (IMAGEHLP.@)
> >  */
> > BOOL WINAPI MapAndLoad(
> >   LPSTR pszImageName, LPSTR pszDllPath, PLOADED_IMAGE pLoadedImage,
> >   BOOL bDotDll, BOOL bReadOnly)
> > {
> >   CHAR szFileName[MAX_PATH];
> >   HANDLE hFile = NULL;
> >   HANDLE hFileMapping = NULL;
> >   HMODULE hModule = NULL;
> >   PIMAGE_NT_HEADERS pNtHeader = NULL;
> >
> > TRACE("(%s, %s, %p, %d, %d)\n", pszImageName, pszDllPath, 
> > pLoadedImage,
> >                                     bDotDll, bReadOnly);
> >     /* PathCombine(&szFileName, pszDllPath, pszImageName); */
> >   /* PathRenameExtension(&szFileName, bDotDll?:"dll":"exe"); */
> >
> > /* FIXME: Check if the file already loaded (use 
> > IMAGEHLP_pFirstLoadedImage) */
> >   if(!(hFile = CreateFileA(
> >     szFileName, GENERIC_READ, 1, /* FIXME: FILE_SHARE_READ not 
> > defined */
> >     NULL, OPEN_EXISTING, 0, NULL)))
> >     {
> >       SetLastError(ERROR_FILE_NOT_FOUND);
> >       goto Error;
> >     }
> >
> > Cheers,
> > Adam
>
> Please file a bug in bugzilla regarding this. wine-devel is not the 
> place to file bug reports, much more can get done in bugzilla.
>
> http://bugs.winehq.org
>
> Thank you.

And yes I saw a bug was already filed about it, I meant if you think it 
might've been a different issue to file a separate report. But if you 
think it's the same bug, then the best you can do is post about it in 
bugzilla, since wine-devel is not a place to report bugs.

Thank you.

[James Hawkins]

On 11/17/06, Aaron Slunt <tonglebeak(a)gmail.com> wrote:
> Adam Connell wrote:
> > Greetings,
> >
> > While investigating why a game (Kasparov's Chessmate) didn't work,
> > I think I found a bug in the implementation of imagehlp.dll in the
> > the MapAndLoad function located in dlls/imagehlp/access.c lines
> > 165-167. The function looks like it tries to open an uninitialized
> > string szFileName instead of the passed file name "pszImageName".
> >
> > I think there has been a bug filed already (Bug 6099 Alien Shooter
> > needs imagehlp.dll override) and I think this explains it.
> >
> > Wine version 0.9.25
> >
> > /***********************************************************************
> >  *            MapAndLoad (IMAGEHLP.@)
> >  */
> > BOOL WINAPI MapAndLoad(
> >   LPSTR pszImageName, LPSTR pszDllPath, PLOADED_IMAGE pLoadedImage,
> >   BOOL bDotDll, BOOL bReadOnly)
> > {
> >   CHAR szFileName[MAX_PATH];
> >   HANDLE hFile = NULL;
> >   HANDLE hFileMapping = NULL;
> >   HMODULE hModule = NULL;
> >   PIMAGE_NT_HEADERS pNtHeader = NULL;
> >
> > TRACE("(%s, %s, %p, %d, %d)\n", pszImageName, pszDllPath,
> > pLoadedImage,
> >                                     bDotDll, bReadOnly);
> >
> > /* PathCombine(&szFileName, pszDllPath, pszImageName); */
> >   /* PathRenameExtension(&szFileName, bDotDll?:"dll":"exe"); */
> >
> > /* FIXME: Check if the file already loaded (use
> > IMAGEHLP_pFirstLoadedImage) */
> >   if(!(hFile = CreateFileA(
> >     szFileName, GENERIC_READ, 1, /* FIXME: FILE_SHARE_READ not defined
> > */
> >     NULL, OPEN_EXISTING, 0, NULL)))
> >     {
> >       SetLastError(ERROR_FILE_NOT_FOUND);
> >       goto Error;
> >     }
> >
> > Cheers,
> > Adam
>
> Please file a bug in bugzilla regarding this. wine-devel is not the
> place to file bug reports, much more can get done in bugzilla.

There's nothing wrong with bringing this up on wine-devel.  Adam has
taken the time to track down the possible bug culprit, and he's
sharing it with the developers.  If, on the other hand, he had just
said, "app xyz doesn't work, don't know why" the the response would be
correct.  Not every developer reads wine-bugs, or checks the bugzilla.

-- 
James Hawkins

[Louis Lenders]

> There's nothing wrong with bringing this up on wine-devel.  Adam has
> taken the time to track down the possible bug culprit, and he's
> sharing it with the developers.  If, on the other hand, he had just
> said, "app xyz doesn't work, don't know why" the the response would be
> correct.  Not every developer reads wine-bugs, or checks the bugzilla.

Totally agreeing with you here. The mail  from Adam Connell has much more to do
with wine development then a load of other really off-topic mails that sometimes
appear on wine-devel mailinglist.