Bug 245971 - [patch sbin/dhclient] remove_protocol() can mangle linked list
Summary: [patch sbin/dhclient] remove_protocol() can mangle linked list
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Mark Johnston
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-27 14:29 UTC by joost
Modified: 2020-06-08 21:38 UTC (History)
3 users (show)

See Also:


Attachments
patch against HEAD (rev 360336) (536 bytes, patch)
2020-04-27 14:29 UTC, joost
no flags Details | Diff
patch against HEAD (rev 360336) (731 bytes, patch)
2020-04-28 15:16 UTC, joost
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description joost 2020-04-27 14:29:38 UTC
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.
Comment 1 Ed Maste freebsd_committer 2020-04-27 14:51:25 UTC
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.
Comment 2 joost 2020-04-27 15:01:09 UTC
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.
Comment 3 joost 2020-04-28 15:15:39 UTC
I see I've attached the wrong patch. I'm attaching the correct one.
Comment 4 joost 2020-04-28 15:16:20 UTC
Created attachment 213889 [details]
patch against HEAD (rev 360336)
Comment 5 Mark Johnston freebsd_committer 2020-06-02 14:40:46 UTC
Ed, do you plan to commit this?  If not I can take it.
Comment 6 commit-hook freebsd_committer 2020-06-04 16:24:34 UTC
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
Comment 7 commit-hook freebsd_committer 2020-06-08 21:36:26 UTC
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
Comment 8 Mark Johnston freebsd_committer 2020-06-08 21:38:09 UTC
Thanks for the report and patch.