This line in reload_gws() is not atomic:
(*lcrs_ws_reload_counter)++;
An atomic counter should be used for that, otherwise the value of
lcrs_ws_reload_counter may end up with the wrong value if 2 concurrent
processes modify it in a narrow time window.
The same is true for the lines above that:
if (*lcrs == lcrs_1) {
lcrs_2[i].end_record = 1;
*lcrs = lcrs_2;
} else {
lcrs_1[i].end_record = 1;
*lcrs = lcrs_1;
}
and the similar lines for gateways:
if (*gws == gws_1) {
gws_2[i].ip_addr = 0;
*gws = gws_2;
} else {
gws_1[i].ip_addr = 0;
*gws = gws_1;
}
Without locking the wrong table pointer could end up in lcrs and gws, or
even worse, mixed lcr and gw table pointers. The whole lcr reload
mechanism is very fragile in this regard if concurrent lcr_reload
commands arrive in narrow time intervals, even though it appears to make
provisions to be able to do 2 reloads in a narrow time window.
IMO all 3 operations presented above should be done together under a lock.
Even better a more robust reload mechanism should be implemented, because
it's very difficult to prevent a provisioning system from issuing the
reload lcr command multiple times when the provisioning interface allows
concurrent access to its resources.
--
Dan
_______________________________________________
Devel mailing list
Devel@lists.openser.org
http://lists.openser.org/cgi-bin/mailman/listinfo/devel
