Fix T55974: Crash when using Surface Force Field Type on UV Sphere
ClosedPublic
Actions

Authored by Andrew Williams (sobakasu) on Jan 13 2019, 10:49 AM.

Details

Reviewers

Sybren A. Stüvel (sybren)
Bastien Montagne (mont29)
Brecht Van Lommel (brecht)

Commits

rBS102631486b48: Fix potential invalid memory access in surface force field BVH tree.
rB102631486b48: Fix potential invalid memory access in surface force field BVH tree.

Summary

fix for https://developer.blender.org/T55974

Diff Detail

Repository: rB Blender

Event Timeline

Andrew Williams (sobakasu) created this revision.Jan 13 2019, 10:49 AM

Andrew Williams (sobakasu) edited the summary of this revision. (Show Details)Jan 13 2019, 10:53 AM

Andrew Williams (sobakasu) removed reviewers: Bastien Montagne (mont29), Sybren A. Stüvel (sybren).

I think i've found the problem that causes this crash. MOD_surface.c deformVerts, calls BKE_id_free on surmd->mesh. This frees the cached tree used by surmd->bvhtree. surmd->bvhtree is not rebuilt until later, so other threads can access the invalid bvhtree. setting surmd->bvhtree to NULL stops this happening.

Andrew Williams (sobakasu) added reviewers: Sybren A. Stüvel (sybren), Bastien Montagne (mont29).Jan 15 2019, 1:09 AM

Andrew Williams (sobakasu) retitled this revision from Fix for T55974 to Fix T55974: Crash when using Surface Force Field Type on UV Sphere.Jan 16 2019, 2:03 AM

Brecht Van Lommel (brecht) added a reviewer: Brecht Van Lommel (brecht).Jan 21 2019, 3:05 AM

The deeper issue here is a race condition due to missing dependencies. But this code is problematic too, so I'll commit a modified version of this patch as well.

source/blender/blenlib/intern/BLI_kdopbvh.c
900	This is not equivalent, it does not use `MEM_SAFE_FREE` in case memory allocation failed.
source/blender/modifiers/intern/MOD_surface.c
101	Blender comment style uses `/* */`.
175–176	If allocation of `surmd->mesh` fails, this bvhtree will not be freed.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 22 2019, 3:26 PM

Closed by commit rB102631486b48: Fix potential invalid memory access in surface force field BVH tree. (authored by Andrew Williams (sobakasu), committed by Brecht Van Lommel (brecht)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Andrew Williams (sobakasu) marked an inline comment as done.Jan 22 2019, 11:02 PM

Andrew Williams (sobakasu) added inline comments.

source/blender/blenlib/intern/BLI_kdopbvh.c
900	this part got committed to master, not sure if that was a mistake or not. thanks for fixing this by the way, now i can use surface force fields again :)