Accuracy of TIFF Field metadata (COLORMAP)

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Accuracy of TIFF Field metadata (COLORMAP)

rleigh
I recently discovered a problem on Windows where I was getting a strange
runtime error ("Run-Time Check Failure #2 -S") which turns out was due
to stack corruption.  This was due to calling TIFFGetField with the
wrong parameters, and that was because the field metadata in
tif_dirinfo.c was wrong.  Here's the broken bit:

         { TIFFTAG_COLORMAP, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_OTHER,
TIFF_SETGET_UNDEFINED, FIELD_COLORMAP, 1, 0, "ColorMap", NULL },

It's the "-1, -1," values which map to TIFF_VARIABLE.  It should be -2
which is TIFF_SPP.  At least, that's my understanding (which could be
wrong).

I wrote a set of type-safe generic tag processing functions in C++ which
make use of the metadata and expect it to be correct.  The logic is
essentially this for the three-array variant:

           int readcount = TIFFFieldReadCount(field);

           typename T::value_type::value_type *valueptr0, *valueptr1,
*valueptr2; // type of array of array of values
           uint32_t count;

           if (readcount == TIFF_SPP)
             {
               uint16_t spp;
               ifd->getRawField(TIFFTAG_SAMPLESPERPIXEL, &spp);
               ifd->getRawField(tag, &valueptr0, &valueptr1, &valueptr2);
               count = static_cast<uint32_t>(spp);
             }
           else if (readcount == TIFF_VARIABLE)
             {
               uint16_t n;
               ifd->getRawField(tag, &n, &valueptr0, &valueptr1,
&valueptr2);
               count = static_cast<uint32_t>(n);
             }
           else if (readcount == TIFF_VARIABLE2)
             {
               ifd->getRawField(tag, &count, &valueptr0, &valueptr1,
&valueptr2);
             }
           else
             {
               if (!limit)
                 ifd->getRawField(tag, &valueptr0, &valueptr1,
&valueptr2);
               else
                 ifd->getRawField(tag, &valueptr0);
               count = static_cast<uint32_t>(readcount);
             }

           // copy valueptr[0-2] + count into destination array(s)

Note: it's using wrappers around libtiff, but getRawField is essentially
TIFFGetField with added locking for thread safety.  
(https://github.com/ome/ome-files-cpp/blob/develop/lib/ome/files/tiff/Field.cpp#L522 
is the actual code and
https://github.com/ome/ome-files-cpp/blob/develop/lib/ome/files/in/OMETIFFReader.cpp#L817 
is an example of its use)

Since I'm hitting the TIFF_VARIABLE case, I'm clearly calling
TIFFGetField with the wrong arguments and this ends up trashing the
stack (since we're storing a 64-bit pointer in a 16-bit type).

Assuming I'm not totally wrong in my understanding, the wider question
here is this: is the field metadata generally usable?  Or does it need a
proper audit of every field to ensure the types are correct in every
case?  Would it help to use the VARIABLE/VARIABLE2/SPP values in place
of negative values?


Regards,
Roger


--
Dr Roger Leigh -- Open Microscopy Environment
Wellcome Trust Centre for Gene Regulation and Expression,
School of Life Sciences, University of Dundee, Dow Street,
Dundee DD1 5EH Scotland UK   Tel: (01382) 386364
_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|

Re: Accuracy of TIFF Field metadata (COLORMAP)

mickey.rose
> I recently discovered a problem on Windows where I was getting a strange
> runtime error ("Run-Time Check Failure #2 -S") which turns out was due
> to stack corruption. This was due to calling TIFFGetField with the
> wrong parameters, and that was because the field metadata in
> tif_dirinfo.c was wrong. Here's the broken bit:
>
> { TIFFTAG_COLORMAP, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_OTHER,
> TIFF_SETGET_UNDEFINED, FIELD_COLORMAP, 1, 0, "ColorMap", NULL },

These values are correct. There's field_passcount=0, which means you
shouldn't be asking for the nunber of colors (&n in your code) from
TIFFGetField. See:

https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L327
https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L939

TIFFGetField gives you the three uint16* pointers to r/g/b arrays,
whose length, i.e. the number of colors in the palette, is determined
by bitspersample (n = 1<<bitspersample)

> Assuming I'm not totally wrong in my understanding, the wider question
> here is this: is the field metadata generally usable? Or does it need a
> proper audit of every field to ensure the types are correct in every
> case? Would it help to use the VARIABLE/VARIABLE2/SPP values in place
> of negative values?

There's a comment at the top acknowledging that symbolic names should
be used, and something about formatting... I assume they'd make the
table a bit harder to skim through.

_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|

Re: Accuracy of TIFF Field metadata (COLORMAP)

rleigh
On 2016-09-19 18:47, [hidden email] wrote:

>> I recently discovered a problem on Windows where I was getting a
>> strange
>> runtime error ("Run-Time Check Failure #2 -S") which turns out was due
>> to stack corruption. This was due to calling TIFFGetField with the
>> wrong parameters, and that was because the field metadata in
>> tif_dirinfo.c was wrong. Here's the broken bit:
>>
>> { TIFFTAG_COLORMAP, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_OTHER,
>> TIFF_SETGET_UNDEFINED, FIELD_COLORMAP, 1, 0, "ColorMap", NULL },
>
> These values are correct. There's field_passcount=0, which means you
> shouldn't be asking for the nunber of colors (&n in your code) from
> TIFFGetField. See:
>
> https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L327
> https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L939
>
> TIFFGetField gives you the three uint16* pointers to r/g/b arrays,
> whose length, i.e. the number of colors in the palette, is determined
> by bitspersample (n = 1<<bitspersample)

I'm not sure I follow completely.

        unsigned char field_passcount; /* if true, pass dir count on set */

This is a "get" operation I'm performing, so I'm not sure it applies.  
Does it also apply to get?

Additionally, where is it specified that the value is
"1<<bitspersample"?  I know it's in the specification e.g.
http://www.awaresystems.be/imaging/tiff/tifftags/colormap.html .  What I
mean is I'm trying to write this code without any specific prior
knowledge of the tag types, and I'd like to obtain that information from
libtiff itself, without making any assumptions in my code.  Is that
possible to obtain in any way?

Is the read count here really valid being TIFF_VARIABLE?  Why would
TIFFFieldReadCount not return the actual size?  i.e. 1<<bitspersample.  
Or would it be appropriate to add

#define TIFF_BPS       -4              /* marker for BitsPerSample
length tags */

and use that instead?

Basically I'm looking for a way to correctly introspect this
information, since I can't see how I could get this with the existing
API, unless I'm not seeing something basic.  Is anything which is
TIFF_VARIABLE or TIFF_VARIABLE2 with passcount==0 automatically assumed
to be 1<<bitspersample in length?  If so, is that universally true, or
are there any exceptions to the rule?

>> Assuming I'm not totally wrong in my understanding, the wider question
>> here is this: is the field metadata generally usable? Or does it need
>> a
>> proper audit of every field to ensure the types are correct in every
>> case? Would it help to use the VARIABLE/VARIABLE2/SPP values in place
>> of negative values?
>
> There's a comment at the top acknowledging that symbolic names should
> be used, and something about formatting... I assume they'd make the
> table a bit harder to skim through.

Maybe if each field was split over two lines it would fit better?


Regards,
Roger
_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|

Re: Accuracy of TIFF Field metadata (COLORMAP)

mickey.rose
On 2016-09-20 11:06, [hidden email] wrote:

> On 2016-09-19 18:47, [hidden email] wrote:
>>> I recently discovered a problem on Windows where I was getting a
>>> strange
>>> runtime error ("Run-Time Check Failure #2 -S") which turns out was due
>>> to stack corruption. This was due to calling TIFFGetField with the
>>> wrong parameters, and that was because the field metadata in
>>> tif_dirinfo.c was wrong. Here's the broken bit:
>>>
>>> { TIFFTAG_COLORMAP, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_OTHER,
>>> TIFF_SETGET_UNDEFINED, FIELD_COLORMAP, 1, 0, "ColorMap", NULL },
>>
>> These values are correct. There's field_passcount=0, which means you
>> shouldn't be asking for the nunber of colors (&n in your code) from
>> TIFFGetField. See:
>>
>> https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L327
>> https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L939
>>
>> TIFFGetField gives you the three uint16* pointers to r/g/b arrays,
>> whose length, i.e. the number of colors in the palette, is determined
>> by bitspersample (n = 1<<bitspersample)
>
> I'm not sure I follow completely.
>
>     unsigned char field_passcount;  /* if true, pass dir count on set */
>
> This is a "get" operation I'm performing, so I'm not sure it applies.  
> Does it also apply to get?

That's my understanding, yes. In _TIFFVGetField it's only used in the
custom field case, probably because the rest are hardcoded.

https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L1053

> Additionally, where is it specified that the value is
> "1<<bitspersample"?  I know it's in the specification e.g.
> http://www.awaresystems.be/imaging/tiff/tifftags/colormap.html .

It's hardcoded in various places:

https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L328
https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dirread.c#L3771
https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dirwrite.c#L1774

These places are not related to TIFFGetField, but that's because
    TIFFGetField(tif, TIFFTAG_COLORMAP, &p0, &p1, &p2);
returns only pointers to arrays, it doesn't care how long they are.

> What I
> mean is I'm trying to write this code without any specific prior
> knowledge of the tag types, and I'd like to obtain that information from
> libtiff itself, without making any assumptions in my code.  Is that
> possible to obtain in any way?

I don't think that's possible. Look at TIFFTAG_COLORMAP and
TIFFTAG_TRANSFERFUNCTION, for example. Their TIFFField records, except
the name, are identical:

{ TIFFTAG_TRANSFERFUNCTION, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_OTHER,
TIFF_SETGET_UNDEFINED, FIELD_TRANSFERFUNCTION, 1, 0, "TransferFunction",
NULL },

Yet their handling in _TIFFVGetField differs:

https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L939

    case TIFFTAG_COLORMAP:
        *va_arg(ap, uint16**) = td->td_colormap[0];
        *va_arg(ap, uint16**) = td->td_colormap[1];
        *va_arg(ap, uint16**) = td->td_colormap[2];

https://github.com/vadz/libtiff/blob/c87dc82d7ff8214bd417176bd76f3fe94a237659/libtiff/tif_dir.c#L1005

    case TIFFTAG_TRANSFERFUNCTION:
        *va_arg(ap, uint16**) = td->td_transferfunction[0];
        if (td->td_samplesperpixel - td->td_extrasamples > 1) {
            *va_arg(ap, uint16**) = td->td_transferfunction[1];
            *va_arg(ap, uint16**) = td->td_transferfunction[2];
        }

_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/