On Thu, 2018-03-15 at 16:55 +0100, Hugo Lefeuvre wrote: [...] > * My understanding of the problem: > > Under certain conditions, the td->td_transferfunction table might not > have the excepted size, that is it may not have the excepted number of > samples per pixel (td->td_samplesperpixel). In this case for example, > the table is only 3 rows large while td->td_samplesperpixel is 4. Then, > the program segfaults when it comes to td->td_transferfunction[3][0]. > > * My understanding of your patch: > > You are introducing a loop which verifies that td->td_transferfunction > has the excepted number of samples per pixel by looking at the pointer > to each row and making sure it is not NULL (which would mean that it is > out of bounds). If it is NULL, you 'disable' the actual loop by setting > n to 0. > > * My understanding of why it might not work with -O1 or -O2, etc.: > > You are assuming that td->td_transferfunction[i] is out of bounds if and > only if it is NULL, which is AFAIK undefined behavior (I couldn't find > any information about it, not 100% sure). While it might be true with > -O0 / some compilers, it might not always be the case. Absolutely, out-of-bounds access has undefined behaviour. The compiler will (in general) optimise on the assumption that the input program never attempts to do anything that's specified as having undefined behaviour, which can have extremely weird results if it does. > If it's not the case, then the if body isn't executed and tiff might > still crash. > > Also, I'm not completely sure this is the best way to solve this problem. > > In fact, while this kind of patch would help avoiding a crash, tiff would > still be broken in some way because we wouldn't really have fixed the original > problem (the fact that the td->td_transferfunction table has an unexpected > size). > > Wouldn't it be better to catch the issue earlier, for example when building > the td->td_transferfunction table or when defining td->td_samplesperpixel, > in order to make this inconsistent state impossible ? Yes. Ben. > (Please correct me if I'm wrong !) > > Hope this helps ! :) -- Ben Hutchings Quantity is no substitute for quality, but it's the only one we've got.
Attachment:
signature.asc
Description: This is a digitally signed message part