Random crash with GetFeatureInfo on raster outside of the extent of the image (valgrind trace inside) #4778

Merged
merged 1 commit into from Oct 9, 2013

Projects

None yet

2 participants

@thefab
thefab commented Oct 3, 2013

Tested with the 6.2.0 and with the current state of the 6.2 branch (I can't update to 6.4 for now).

There are some memory errors with valgrind in this particular case (so a random crash). Valgrind test is ok when the GetFeatureInfo point is inside the extent of the raster.

Here is the bad valgrind trace:

==4245== Memcheck, a memory error detector
==4245== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==4245== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==4245== Command: mapserv QUERY_STRING=export_geometry=false&y=339&layers=snow_probability&bbox=-50.02584162855885%2C-54.67346707199171%2C179.9%2C80.74744935922759&styles=&service=WMS&x=721&transparent=TRUE&format=image%2Fpng&request=GetFeatureInfo&width=876&feature_count=1&srs=EPSG%3A4326&exceptions=application%2Fvnd.ogc.se_inimage&version=1.1.1&height=516&info_format=application%2Fjson&export_links=1&query_layers=snow_probability&time=2013-10-02T09%3A45%3A00Z&xsuid=1&map=/home/synserv/toto.map
==4245== 
[Thu Oct  3 16:46:55 2013].195760 msProjectRect(): Warning: degenerate rect {139.348011,-8.352165,139.348011,-8.352165}
==4245== Invalid read of size 4
==4245==    at 0x4E0DF1C: msRasterQueryByPoint (maprasterquery.c:1102)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245==  Address 0x96a08f0 is 0 bytes inside a block of size 216 free'd
==4245==    at 0x4A072BA: free (vg_replace_malloc.c:446)
==4245==    by 0x4E0B33C: msRasterLayerInfoFree (maprasterquery.c:167)
==4245==    by 0x4E0DB07: msRasterQueryByRect (maprasterquery.c:960)
==4245==    by 0x4E0DF14: msRasterQueryByPoint (maprasterquery.c:1101)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245== 
==4245== Invalid write of size 4
==4245==    at 0x4E0DF85: msRasterQueryByPoint (maprasterquery.c:1115)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245==  Address 0x96a0998 is 168 bytes inside a block of size 216 free'd
==4245==    at 0x4A072BA: free (vg_replace_malloc.c:446)
==4245==    by 0x4E0B33C: msRasterLayerInfoFree (maprasterquery.c:167)
==4245==    by 0x4E0DB07: msRasterQueryByRect (maprasterquery.c:960)
==4245==    by 0x4E0DF14: msRasterQueryByPoint (maprasterquery.c:1101)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245== 
==4245== Invalid read of size 4
==4245==    at 0x4E0DF98: msRasterQueryByPoint (maprasterquery.c:1118)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245==  Address 0x96a08fc is 12 bytes inside a block of size 216 free'd
==4245==    at 0x4A072BA: free (vg_replace_malloc.c:446)
==4245==    by 0x4E0B33C: msRasterLayerInfoFree (maprasterquery.c:167)
==4245==    by 0x4E0DB07: msRasterQueryByRect (maprasterquery.c:960)
==4245==    by 0x4E0DF14: msRasterQueryByPoint (maprasterquery.c:1101)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245== 
==4245== Invalid write of size 4
==4245==    at 0x4E0DFA8: msRasterQueryByPoint (maprasterquery.c:1119)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245==  Address 0x96a08fc is 12 bytes inside a block of size 216 free'd
==4245==    at 0x4A072BA: free (vg_replace_malloc.c:446)
==4245==    by 0x4E0B33C: msRasterLayerInfoFree (maprasterquery.c:167)
==4245==    by 0x4E0DB07: msRasterQueryByRect (maprasterquery.c:960)
==4245==    by 0x4E0DF14: msRasterQueryByPoint (maprasterquery.c:1101)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245== 
==4245== Invalid write of size 4
==4245==    at 0x4E0DFF5: msRasterQueryByPoint (maprasterquery.c:1125)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245==  Address 0x96a08fc is 12 bytes inside a block of size 216 free'd
==4245==    at 0x4A072BA: free (vg_replace_malloc.c:446)
==4245==    by 0x4E0B33C: msRasterLayerInfoFree (maprasterquery.c:167)
==4245==    by 0x4E0DB07: msRasterQueryByRect (maprasterquery.c:960)
==4245==    by 0x4E0DF14: msRasterQueryByPoint (maprasterquery.c:1101)
==4245==    by 0x4D3A789: msQueryByPoint (mapquery.c:1608)
==4245==    by 0x4D791C2: msWMSFeatureInfo (mapwms.c:3996)
==4245==    by 0x4D7C9FC: msWMSDispatch (mapwms.c:4992)
==4245==    by 0x4DBB5D3: msOWSDispatch (mapows.c:244)
==4245==    by 0x4E4669C: msCGIDispatchRequest (mapservutil.c:1607)
==4245==    by 0x401129: main (mapserv.c:259)
==4245== 
[Thu Oct  3 16:46:55 2013].418330 msQueryByPoint(): Search returned no results. No matching record(s) found.
Content-Type: application/json

==4245== 
==4245== HEAP SUMMARY:
==4245==     in use at exit: 10,742 bytes in 23 blocks
==4245==   total heap usage: 2,755 allocs, 2,732 frees, 1,012,224 bytes allocated
==4245== 
==4245== LEAK SUMMARY:
==4245==    definitely lost: 152 bytes in 11 blocks
==4245==    indirectly lost: 38 bytes in 2 blocks
==4245==      possibly lost: 0 bytes in 0 blocks
==4245==    still reachable: 10,552 bytes in 10 blocks
==4245==         suppressed: 0 bytes in 0 blocks
==4245== Rerun with --leak-check=full to see details of leaked memory
==4245== 
==4245== For counts of detected and suppressed errors, rerun with: -v
==4245== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 7 from 7)
@tbonfort
Member
tbonfort commented Oct 4, 2013

@thefab could you provide a testcase for this please? Ideally, please use our testsuite and provide a testcase in https://github.com/mapserver/msautotest/blob/master/wxs/wms_raster.map that reproduces the issue.

@thefab
thefab commented Oct 7, 2013

@tbonfort

very easy to reproduce with your map file and your toronto.tif test file:

Just go to the wxs subdir and execute (for example).

valgrind mapserv QUERY_STRING="layers=road&FORMAT=image%2Fpng&SERVICE=WMS&VERSION=1.1.1&REQUEST=GetFeatureInfo&QUERY_LAYERS=road&STYLES=&EXCEPTIONS=application%2Fvnd.ogc.se_inimage&srs=epsg:4326&BBOX=-104.06,44.65,-93.93,44.7&WIDTH=1200&HEIGHT=600&x=100&y=100&map=wms_raster.map"

[...]
==6234== 
==6234== HEAP SUMMARY:
==6234==     in use at exit: 10,552 bytes in 10 blocks
==6234==   total heap usage: 2,826 allocs, 2,816 frees, 897,684 bytes allocated
==6234== 
==6234== LEAK SUMMARY:
==6234==    definitely lost: 0 bytes in 0 blocks
==6234==    indirectly lost: 0 bytes in 0 blocks
==6234==      possibly lost: 0 bytes in 0 blocks
==6234==    still reachable: 10,552 bytes in 10 blocks
==6234==         suppressed: 0 bytes in 0 blocks
==6234== Rerun with --leak-check=full to see details of leaked memory
==6234== 
==6234== For counts of detected and suppressed errors, rerun with: -v
==6234== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 7 from 7)

No error if the GetFeatureInfo is inside the TIFF.

@tbonfort tbonfort added a commit to tbonfort/mapserver that referenced this pull request Oct 9, 2013
@tbonfort tbonfort Fix memory corruption on empty raster query (#4778) 36cac89
@tbonfort tbonfort was assigned Oct 9, 2013
@tbonfort tbonfort added a commit to mapserver/msautotest_DEPRECATED that referenced this pull request Oct 9, 2013
@tbonfort tbonfort add empty raster query test 81e41e7
@tbonfort tbonfort merged commit e83a0cb into mapserver:branch-6-2 Oct 9, 2013

1 check passed

default The Travis CI build passed
Details
@tbonfort tbonfort added a commit to mapserver/msautotest_DEPRECATED that referenced this pull request Oct 9, 2013
@tbonfort tbonfort add empty raster query test f497f67
@tbonfort tbonfort added a commit to mapserver/msautotest_DEPRECATED that referenced this pull request Oct 9, 2013
@tbonfort tbonfort add empty raster query test 30a4ba3
@tbonfort
Member
tbonfort commented Oct 9, 2013

fix applied to branch-6-2, branch-6-4 and master

@thefab
thefab commented Oct 10, 2013

Tested here with branch-6-2, it works like a charm ! many thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment