Bug 247562

Summary: dhclient: service script does not create missing piddir
Product: Base System Reporter: Walter von Entferndt <walter.von.entferndt>
Component: miscAssignee: Chris Rees <crees>
Status: Closed FIXED    
Severity: Affects Many People CC: crees, jbeich, markj, parakleta
Priority: --- Keywords: patch
Version: 12.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch for /etc/rc.d/dhclient (FreeBSD-12.1-RELEASE) none

Description Walter von Entferndt 2020-06-26 13:15:45 UTC
Created attachment 215957 [details]
patch for /etc/rc.d/dhclient (FreeBSD-12.1-RELEASE)

I consider it good practice to have /var/run on a small tmpfs.  The side effect is that this is empty after every reboot.  The advantage is that this unveils bugs otherwise undetected.

Like some port's service scripts, dhclient's /etc/rc.d/dhclient does not check for the existence of it's piddir, and fails to work correctly if it does not exist.

Many users who run into this bug will create the missing directory manually and not report it...

Suggestion:
1. since I encountered this kind of bug from time to time, I'd like to suggest to add to rc.subr the functionality to create $required_dirs and set $xxx_dir_mode and $xxx_dir_owner (should understand owner:group syntax).
2. /etc/rc should check /var on startup against the mtree file and create missing directories.

The patch appended is against 12.1-RELEASE, and was produced with "patch -U 7" from the root directory.

Thx.
Comment 1 Duane 2021-03-02 01:55:49 UTC
I want to add that I just tripped over this bug as well.

It might be better in the case of dhclient to just use the more common pidfile location of `/var/run/${name}.${ifn}.pid` or `/var/run/${name}-${ifn}.pid`.  This doesn't require the directory to be created.
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2021-03-02 15:26:49 UTC
As I understand, this is fixed by https://cgit.freebsd.org/src/commit/?id=d27999e51396b301d83bb7d6cdd51f6d6f4fff2d

I believe we have enough time to get this into 13.0.
Comment 3 Chris Rees freebsd_committer freebsd_triage 2021-03-02 16:40:24 UTC
Thanks for finding this Mark-- I guess I need to discuss this with re@?
Comment 4 Mark Johnston freebsd_committer freebsd_triage 2021-03-02 16:50:08 UTC
(In reply to Chris Rees from comment #3)
Right.  It should be routine:
- cherry-pick the change to stable/13 (i.e., MFC)
- cherry-pick the stable/13 rev to releng/13.0 and submit a request to re@ following https://wiki.freebsd.org/Releng/ChangeRequestGuidelines
- upon approval, amend the releng/13.0 commit to add "Approved by: re" and push.

Please let me know if I can help.
Comment 5 Chris Rees freebsd_committer freebsd_triage 2021-03-02 19:37:04 UTC
Thanks for the patch- I came across this bug independently and Mark helped me to get that in.

Our solutions do the same thing really.  I'm minded not to add this to rc.subr as it's pretty common idiom to use install -d.  I'll look at getting rclint to check.