RFR: 8264666: Change implementation of safeAdd/safeMult in the LCMSImageLayout class [v2]

Phil Race prr at openjdk.java.net
Wed Apr 27 18:46:02 UTC 2022


On Fri, 22 Apr 2022 07:36:43 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Description of the new version of the fix:
>> While I have worked on this change and tried to consider the comments, I have found that the usages of the "safeAdd/safeMult" in the LCMSImageLayout class are incorrect. Both methods are based on the "Math" versions but throw a different exception. The problem is that its implementation may accept the negative values during intermediate calculation, see the old implementation of "[verify](https://github.com/openjdk/jdk/blob/139615b1815d4afd3593536d83fa8b25430f35e7/src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSImageLayout.java#L343)" method:
>>  1. We check the "offset" value: 0 <= offset <  dataArrayLength
>>  2. We do some intermediate calculations that "accept" negative values
>>  3. We check the final "off" value: 0 <= offset <  dataArrayLength
>> 
>> I wondered is it possible to provide some data that using wrong/negative data at step2 may result in the correct check at step3. I spent some time and was able to reproduce the problem with the attached test case. Note that the test is a little bit cryptic since it is not possible to reproduce it by input image data.
>> 
>> Note: I have removed all cleanup from the fix, to make it simpler.
>> 
>> <======>
>> Description of the old version of the fix:
>> - The hand-crafted methods for addition and multiplication are replaced by the "Math" versions.
>> - Cleanup: the usage of do/while(false) is removed
>
> Sergey Bylokhov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - Initial version 8264666
>  - Merge branch 'master' into LCMSImageLayout
>  - Revert "safeXX -> xxExact"
>    
>    This reverts commit a1876fa596a6831f036c04f45fa89c2caba47087.
>  - Revert "delete "do{...} while (false);""
>    
>    This reverts commit a9e91601c355f96c82dd8b2b8564a3a3e7b96aef.
>  - delete "do{...} while (false);"
>  - safeXX -> xxExact

Marked as reviewed by prr (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3333



More information about the client-libs-dev mailing list