Kinfu MarchingCubes writes unbounded data to hardcode sized buffer

[nh2]

pcl::gpu::MarchingCubes::run:

  if (triangles_buffer.empty())
    triangles_buffer.create(DEFAULT_TRIANGLES_BUFFER_SIZE);

DEFAULT_TRIANGLES_BUFFER_SIZE = 2 * 1000 * 1000 * POINTS_PER_TRIANGLE

marching_cubes.cu/store_point():

triangles_buffer[index] = ...; // index is dependent on how many voxels we have

---

I suggest changing DEFAULT_TRIANGLES_BUFFER_SIZE to WHY_WOULD_ANYONE_EVER_HAVE_MORE_TRIANGLES, that will make the out of bounds crash on the GPU much easier to find.

I also like how it starts with 2 * - that looks almost like somebody thought "hmm for some reason the code works if I make this bigger".

[jspricke]

Hi Niklas, feel free to send a pull request, if you think it's a better name (maybe pick one not as harsh as the one you proposed ;) ). Otherwise I will close this one, as it's not a bug.

[nh2]

Hey Jochen, of course I was joking. The bug is that the size of the buffer should not be hardcoded in the first place.

The kernel should check whether the bound has been reached before writing to the buffer, so that at least we get an error message instead of a crash.

A better alternative would be growing the buffer as needed. I might try an implementation of that in a few weeks, and after I've pull requested my colour version of marching cubes.

[jspricke]

:) +1 for a fix.

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[nh2]

Form a quick search in the code, this issue remains unchanged.

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

[nh2]

Please, this issue will not go away by time...

[haritha-j]

Bit of an unrealistic request after 6 years, but would you happen to still have any code that could reproduce this issue?

[stale[bot]]

Marking this as stale due to 30 days of inactivity.

[nh2]

would you happen to still have any code that could reproduce this issue?

@haritha-j Why would that be necessary?

The code https://github.com/PointCloudLibrary/pcl/blob/5f93fea06da2d2a80a179ccdc5fa8020041d428e/gpu/kinfu/include/pcl/gpu/kinfu/marching_cubes.h#L62

seems pretty clear about things being just as they were 6 years ago: Hardcoded buffer size, and no check in the code that the output will actually fit.

[stale[bot]]

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.