Bug 691876

Summary: typo in imagesmooth.c ?
Product: MuPDF Reporter: zeniko
Component: fitzAssignee: Robin Watts <robin.watts>
Status: RESOLVED FIXED    
Severity: normal CC: robin.watts
Priority: P4    
Version: unspecified   
Hardware: PC   
OS: Windows 7   
URL: http://code.google.com/p/sumatrapdf/issues/detail?id=1162
Customer: Word Size: ---
Attachments: DEBUG_SCALING dump

Description zeniko 2011-01-10 22:37:27 UTC
Could http://code.google.com/p/sumatrapdf/source/diff?spec=svn2625&r=2625&format=side&path=/trunk/mupdf/draw/imagesmooth.c#sc_svn2614_928 be a typo? It's currently apparently possible for min to become less than src_w at line 928 which leads to a negative index at line 934 and thus a potential read access violation. Making that change didn't even regress anything in my test suite.
Comment 1 Robin Watts 2011-01-12 00:28:35 UTC
No, it's correct as is - or at least should be.

It shouldn't be possible for min to be less than src_w. If it is, then there is clearly something going on that I haven't considered.

If you are in a position to reproduce the crash, I'd very much like to know the values of the arguments to scale_single_col (and the values of the members of weights).

Or even better, build with 'DEBUG_SCALING' defined and get me a dump of the output for the scale operation that crashes.

Thanks.
Comment 2 zeniko 2011-01-12 14:20:14 UTC
Created attachment 7116 [details]
DEBUG_SCALING dump

The attached dump has been produced while rendering the first page of the document downloadable from http://code.google.com/p/sumatrapdf/issues/detail?id=1162 .
Comment 3 zeniko 2011-01-12 14:25:43 UTC
BTW: You should be able to reproduce the potential read access violation yourself, even if it doesn't lead to a crash: just set a conditional breakpoint at line 928 on "(src_w-min)*n>src_w".
Comment 4 Robin Watts 2011-01-12 18:13:10 UTC
Having looked more closely at the code, I do:

   src_w = (src_w-1)*n;

earlier, hence src_w has already been multiplied by n. You are therefore absolutely correct, and I shouldn't be multiplying src_w by n again. Your fix was ideal. I've asked Tor to push it to the repo.

Thanks for this!
Comment 5 Robin Watts 2011-01-17 11:46:40 UTC
Tor has now recorded this patch, so closing. Thanks all.