Bug 245971
| Summary: | [patch sbin/dhclient] remove_protocol() can mangle linked list | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | joost | ||||||
| Component: | bin | Assignee: | Mark Johnston <markj> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | cem, emaste, markj | ||||||
| Priority: | --- | ||||||||
| Version: | CURRENT | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Thank you for the patch. Out of curiosity, how did you discover the issue - i.e., was it because of an observed failure, by reviewing code, use of static analysis tools, etc. You could call it code review. I'm trying to implement on-demand renewals, similar to windows `ipconfig /renew`. While doing that I came across remove_protocol(), the looks of which didn't feel right. So I started digging. I see I've attached the wrong patch. I'm attaching the correct one. Created attachment 213889 [details]
patch against HEAD (rev 360336)
Ed, do you plan to commit this? If not I can take it. A commit references this bug: Author: markj Date: Thu Jun 4 16:24:13 UTC 2020 New revision: 361793 URL: https://svnweb.freebsd.org/changeset/base/361793 Log: dhclient: Fix a logic bug remove_protocol(). A logic bug in remove_protocol() meant that it would remove (leak) all structures in the list preceding the one intended for removal. PR: 245971 Submitted by: joost@jodocus.org (original version) MFC after: 1 week Changes: head/sbin/dhclient/dispatch.c A commit references this bug: Author: markj Date: Mon Jun 8 21:35:24 UTC 2020 New revision: 361941 URL: https://svnweb.freebsd.org/changeset/base/361941 Log: MFC r361793: dhclient: Fix a logic bug remove_protocol(). PR: 245971 Changes: _U stable/12/ stable/12/sbin/dhclient/dispatch.c Thanks for the report and patch. |
Created attachment 213856 [details] patch against HEAD (rev 360336) The remove_protocol() function is meant to remove one specific entry from a singly-linked list. While removing the entry, the next entry is set as the start of the list, disregarding all entries before the one being removed.