Bug #42834 for dhcp-public: Problem with omapi_iscsock_cb(omapip/dispatch.c)

On Tue Jul 12 12:39:52 2016, pinjalaganeshb@gmail.com wrote: > Hi, > > This is Ganesh P , Would like to know whether the look-up logic of > omapi_io_states is appropriate when picking up the correct state > information for the given ( input ) cbarg. > > Existing Logic: > > omapi_iscsock_cb(isc_task_t *task, > isc_socket_t *socket, > void *cbarg, > int flags) > { > #if SOCKDELETE > /* > * walk through the io states list, if our object is on there > * service it. if not ignore it. > */ > for (obj = omapi_io_states.next; (obj != NULL) ; obj = obj->next) { I think there may have been a cut and paste error. In my copy of the code the check statement is (obj != NULL) && (obj->next != NULL); > if (obj == cbarg) { > break; > } > } > > if (obj == NULL) { > return(0); > } > #else > } > > "obj = obj->next" is this condition to be removed from for loop ? because > of having it we might never do a comparison of last state of > "omapi_io_states" , isn't it ? I think you meant to refer to the (obj->next != NULL) condition rather than the obj = obj->next statement. It does appear to be incorrect. I'll have to review the code more deeply to see if there is any reason for it to be there. > > in existing code flow , failing to find an appropriate > "omapi_io_states" ( i.e no match with "cbarg" ) can lead to invocation of > "status = obj->reader(obj->inner);" and hence "dhcp" process might wait on > an UNSET FD and blocks forever ? I think this turns out to be only possibly in theory. I think that in reality the matching socket is always on the list. We either find it by comparison with cbarg, or it is the last one on the list and we default to using it. That is, there appears to be a bug in the code but as the rest of the code is generally well behaved this bug probably doesn't cause an issue. Have you seen any problems in this area? > > Can you please clarify ? > > Thanks in advance , > > Ganesh.

---

Hello Ganesh: You'll be pleased to learn that we've addressed your issue for our next major release of ISC_DHCP, 4.4.0. The release date hasn't been finalized at this time. The solution was to simply remove the check of (obj->next != NULL) from the for-loop expresion. It is our custom to credit our contributors by citing them in the release notes by name and/or organization. If you wish to be recognized in this fashion please respond with how you would like to be identified. Thank you for taking the time and interest to report this issue, we will welcome any future submissions you wish to make. Sincerely, Thomas Markwalder ISC Software Engineering

---