You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by Marvin Humphrey <ma...@rectangular.com> on 2016/03/01 04:39:55 UTC

Re: [lucy-dev] Please review JIRA issues

On Sat, Feb 6, 2016 at 8:05 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> Lucifers,
>
> I just cleaned up lots of JIRA issues for Lucy and Clownfish. Please review
> the remaining unresolved issues, especially if you're the assignee.

I've now reviewed all the issues for both Lucy and Clownfish,
resolving or postponing all except one:

    https://issues.apache.org/jira/browse/CLOWNFISH-37
    Order of refcount manipulation when overwriting

I think we should probably get to this one before releasing 0.5.0.  It
has the potential to cause memory errors for the C bindings, and the C
bindings are becoming more prominent.

Marvin Humphrey

Re: [lucy-dev] Please review JIRA issues

Posted by Nick Wellnhofer <we...@aevum.de>.
On 02/03/2016 00:43, Marvin Humphrey wrote:
> I guess I would describe it as low-risk rather than harmless. I can
> imagine some reasonable code paths that would trigger the error, but I
> can accept that they are somewhat esoteric.

Yes, thinking more about it, there's a risk when setters are used with 
non-incremented return values. When working with non-incremented values, you 
must never do anything that might lead to the returning object losing its 
reference to the value. So the following is obviously wrong:

     // SomeClass_Get_Attr returns a non-incremented value.
     Obj *attr = SomeClass_Get_Attr(obj);
     SomeClass_Set_Attr(obj, other_attr);
     // You can't continue to use attr, because the call to
     // Set_Attr might have destroyed it.

But in the following case, it's not so obvious:

     Obj *attr = SomeClass_Get_Attr(obj);
     // Some harmless computations that might change attr
     // or not.
     SomeClass_Set_Attr(obj, attr);

Here the call to SomeClass_Set_Attr can cause a use-after-free if this setter 
is written in the problematic style mentioned in CLOWNFISH-37.

Reviewing the Clownfish codebase to fix all the setters shouldn't take much 
time. I'll simply go ahead and do it.

Nick

Re: [lucy-dev] Please review JIRA issues

Posted by Marvin Humphrey <ma...@rectangular.com>.
On Tue, Mar 1, 2016 at 10:32 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> On 01/03/2016 04:39, Marvin Humphrey wrote:
>>
>>      https://issues.apache.org/jira/browse/CLOWNFISH-37
>>      Order of refcount manipulation when overwriting
>>
>> I think we should probably get to this one before releasing 0.5.0.  It
>> has the potential to cause memory errors for the C bindings, and the C
>> bindings are becoming more prominent.
>
>
> I came to the conclusion that this is harmless. See my comment on the issue.

I guess I would describe it as low-risk rather than harmless. I can
imagine some reasonable code paths that would trigger the error, but I
can accept that they are somewhat esoteric.  OK, let's not block 0.5.0
on this bug.

Marvin Humphrey

Re: [lucy-dev] Please review JIRA issues

Posted by Nick Wellnhofer <we...@aevum.de>.
On 01/03/2016 04:39, Marvin Humphrey wrote:
>      https://issues.apache.org/jira/browse/CLOWNFISH-37
>      Order of refcount manipulation when overwriting
>
> I think we should probably get to this one before releasing 0.5.0.  It
> has the potential to cause memory errors for the C bindings, and the C
> bindings are becoming more prominent.

I came to the conclusion that this is harmless. See my comment on the issue.

Nick