#47448 Segfault in 389-ds-base-1.3.1.4-1.fc19 when setting up FreeIPA replication
Closed: wontfix None Opened 10 years ago by pviktori.

The latest 389-ds from updates-testing reliably (for me) crashes when a FreeIPA replica is being set up. I get the following in system logs:

Jul 24 18:02:22 vm-136.idm.lab.eng.brq.redhat.com kernel: ns-slapd[12544]: segfault at 7f5bb595a630 ip 00007f594a9e9693 sp 00007f59207eb5c0 error 4 in libslapd.so.0.0.0[7f594a929000+101000]


Linked to Bugzilla bug: https://bugzilla.redhat.com/show_bug.cgi?id=987705 (''Red Hat Enterprise Linux 7'')

I could reproduce the crash locally without IPA. Steps:

Init replication with the attached file repl-init-100.ldif

Run the the attached script mod-emp.sh in this sequence:
let i=1;
while [ $i -lt 101 ]
do
./mod-emp.sh $i > /dev/null
./mod-emp.sh $i > /dev/null
./mod-emp.sh $i > /dev/null
let i=$i+1;
done

The fix looks good.

I have one question: The problem occurs here since it looks it's guaranteed at the other 2 places valueset_add_valueset is called with empty v1?
529 entry_delete_present_values_wsi(Slapi_Entry e, const char type, struct berval vals, const CSN *csn, i nt urp, int mod_op, struct berval replacevals)
548 if(!slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE))
549 {
550 / We don't maintain a deleted value list for single valued attributes /
551 valueset_add_valueset(&a->a_deleted_values, &a->a_present_values); / JCM Would be be tter to passin the valuestodelete /
552 }

Replying to [comment:7 nhosoi]:

The fix looks good.

I have one question: The problem occurs here since it looks it's guaranteed at the other 2 places valueset_add_valueset is called with empty v1?
529 entry_delete_present_values_wsi(Slapi_Entry e, const char type, struct berval vals, const CSN *csn, i nt urp, int mod_op, struct berval replacevals)
548 if(!slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE))
549 {
550 / We don't maintain a deleted value list for single valued attributes /
551 valueset_add_valueset(&a->a_deleted_values, &a->a_present_values); / JCM Would be be tter to passin the valuestodelete /
552 }

Yes, the problem is because vs1 is not empty - a->a_deleted_values can be non-empty at this point.

There are two other places where valueset_add_valueset is called. slapi_valueset_set_valueset needs to be fixed too, although it does not cause a crash, it could cause a leak - slapi_valueset_init does not free the contents, it just sets everything to 0/NULL, which could leak. That should be fixed in a separate ticket. The last place valueset_add_valueset is called is in slapi_valueset_join_attr_valueset - that call is safe because it first checks to see if slapi_valueset_isempty.

Replying to [comment:8 rmeggins]:

Yes, the problem is because vs1 is not empty - a->a_deleted_values can be non-empty at this point.

There are two other places where valueset_add_valueset is called. slapi_valueset_set_valueset needs to be fixed too, although it does not cause a crash, it could cause a leak - slapi_valueset_init does not free the contents, it just sets everything to 0/NULL, which could leak.
I searched all the code and found all the callers of slapi_valueset_set_valueset pass newly initialized v1. It looks it was the author's intention... So, for now there is no leak in our server. But of course, it's nice to fix it to prevent the future leaks... ;)

That should be fixed in a separate ticket. The last place valueset_add_valueset is called is in slapi_valueset_join_attr_valueset - that call is safe because it first checks to see if slapi_valueset_isempty.

Replying to [comment:9 nhosoi]:

Replying to [comment:8 rmeggins]:

Yes, the problem is because vs1 is not empty - a->a_deleted_values can be non-empty at this point.

There are two other places where valueset_add_valueset is called. slapi_valueset_set_valueset needs to be fixed too, although it does not cause a crash, it could cause a leak - slapi_valueset_init does not free the contents, it just sets everything to 0/NULL, which could leak.
I searched all the code and found all the callers of slapi_valueset_set_valueset pass newly initialized v1. It looks it was the author's intention... So, for now there is no leak in our server. But of course, it's nice to fix it to prevent the future leaks... ;)

Yes - it's easy to mis-interpret the pre-conditions. That's going into an upcoming ticket.

That should be fixed in a separate ticket. The last place valueset_add_valueset is called is in slapi_valueset_join_attr_valueset - that call is safe because it first checks to see if slapi_valueset_isempty.

The plugin guide is quite clear about slapi_valueset_set_valueset():

43.12. slapi_valueset_set_valueset()
Description
This function initializes a Slapi_ValueSet structure by copying the values contained in another Slapi_ValueSet structure.

and warns that the caller has to take care vs1 is empty.

So there is this call in entrywsi, which really wants to add a valuest to an existing one. This call is gone in master as effect of #569.
For 1.3.1 I suggest the changes in the attached patch

I did test ipa replica install with Rich's suggested fix and it didn't crash anymore

To ssh://git.fedorahosted.org/git/389/ds.git
857029b..df53a87 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit df53a87
Author: Rich Megginson rmeggins@redhat.com
Date: Mon Jul 29 12:36:02 2013 -0600
1cc8ede..ed89524 master -> master
commit ed89524
Author: Rich Megginson rmeggins@redhat.com
Date: Mon Jul 29 12:36:02 2013 -0600

This fix is necessary for "Ticket #346 - Slow ldapmodify operation time for large quantities of multi-valued attribute values" including the backported version 1.2.11.

Thanks to Rich for reviewing the backported patch.

Pushed to 389-ds-base-1.2.11 branch:
1ed4222..d36f7ea 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit d36f7ea

Metadata Update from @rmeggins:
- Issue set to the milestone: 1.2.11.29

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/785

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata