Bug 247562 - dhclient: service script does not create missing piddir
Summary: dhclient: service script does not create missing piddir
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Chris Rees
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2020-06-26 13:15 UTC by Walter von Entferndt
Modified: 2021-03-02 19:37 UTC (History)
4 users (show)

See Also:


Attachments
patch for /etc/rc.d/dhclient (FreeBSD-12.1-RELEASE) (1001 bytes, patch)
2020-06-26 13:15 UTC, Walter von Entferndt
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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 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.