#588 Bug in IsNexusInCore()

[Ilsa Faust]

In magick/pixel_cache.c/IsNexusInCore(), don't check for the nexus in core if cache_info->pixels is NULL:

  if (cache_info->type == PingCache)
    {
      /*
        Some coders *do* read the pixels in 'ping' mode.  Skip sync on
        such pixels.
      */
      status=MagickPass;
    }
  else
    if (cache_info->pixels != NULL)
    {
      magick_off_t
        offset;

      offset=nexus_info->region.y*
        (magick_off_t) cache_info->columns+nexus_info->region.x;
      if (nexus_info->pixels == (cache_info->pixels+offset))
        status=MagickPass;
    }

[Bob Friesenhahn]

In the code I am reading, this function is only ever called if cache_info->pixels is not NULL. Do you have evidence that this function is ever called with cache_info->pixels NULL?

[Ilsa Faust]

Add

assert(cache_info->pixels != NULL);

to magick/pixel_cache.c/IsNexusInCore(), now try these commands:

$ gm convert logo: -resize 1024x5000! logo.png
$ gm convert -limit memory 32Mib -limit map 32Mib logo.png logo.png -append append.png

We get:

$ gm convert -limit memory 32Mib -limit map 32Mib logo.png logo.png -append append.png
gm: magick/pixel_cache.c:570: IsNexusInCore: Assertion `cache_info->pixels != NULL' failed.
gm convert: abort due to signal 6 (SIGABRT) "Abort"...
Abort (core dumped)

[Bob Friesenhahn]

I tried your sample commands and the null pointer issue did not occur.
It may well occur with older versions of GraphicsMagick.

What version of GraphicsMagick are you testing with?

Bob

Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt

[Ilsa Faust]

What version of GraphicsMagick are you testing with?

$ gm -version
GraphicsMagick 1.4 snapshot-20181220 Q16 http://www.GraphicsMagick.org/

The assertion is triggered only for a pixel cache on disk:

18:47:51 0:01 0.000u 13859 pixel_cache.c/OpenCache/3486/Cache:
    open logo.png[0] (/tmp/gm4UDdLv[4], disk, 39.1Mi) storage_class=DirectClass, colorspace=RGB

[Bob Friesenhahn]

The use of cache_info->pixels in IsNexusInCore() was only for arithmetic and not a dereference so it is not clear what caused the crash you are seeing. Without adding the assert, I was never able to reproduce a crash. Changeset 15875:733b7e6c2589 moves the IsNexusInCore() code into SetNexus() and adds the check you suggested.