| Message ID | 20211018081200.eptbh3cncnok73yt@email |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | dnsmasq: procd-ujail: workaround startup failure, when leasefile location is in /tmp | expand |
On Mon, Oct 18, 2021 at 08:12:00AM +0000, Bastian Bittorf wrote: > introduced with 44f694ba1bca1417d24e851c637c284f9f78c06d > ("build: select procd-ujail if !SMALL_FLASH") dnsmasq fails > to startup when the leasefile is configured to be in /tmp, > which is just not suited for beeing a jail location. > > With this patch we explicit call jail_mount_rw() for the > (now autocreated) leasedir and show a warning in syslog > for the special case when leasefile is in directory /tmp > > without this patch, the syslog shows: > Thu Oct 14 18:32:38 2021 user.err : jail: creat(/tmp/ujail-lhNbFK/tmp/dhcp.leases) failed: Read-only file system > Thu Oct 14 18:32:38 2021 daemon.crit dnsmasq[1]: cannot open or create lease file /tmp/dhcp.leases: Read-only file system > Thu Oct 14 18:32:38 2021 daemon.crit dnsmasq[1]: FAILED to start up > > This is v2 of the patch with a more correct description what is does. > > Ref: https://bugs.openwrt.org/index.php?do=details&task_id=4085 > Suggested-by: Daniel Golle <daniel@makrotopia.org> I have neither Ack-ed nor suggested this change. The problem here (according to the ticket) is the option resolvfile dhcp.@dnsmasq[0].resolvfile='/tmp/resolv.conf.auto' which should be dhcp.@dnsmasq[0].resolvfile='/tmp/resolv.conf.d/resolv.conf.auto' for things to work well by default. Moving it back to /tmp (for which ever reason) will break things in the way described here as then resolvdir == leasedir which causes the problem. Maybe we should just add a warning for that ("resolvfile and leasefile cannot be in the same directory, please change your configuration.") > Signed-off-by: Bastian Bittorf <bb@npl.de> > --- > .../services/dnsmasq/files/dnsmasq.init | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init > index 3250b2179b..af2effdb26 100644 > --- a/package/network/services/dnsmasq/files/dnsmasq.init > +++ b/package/network/services/dnsmasq/files/dnsmasq.init > @@ -616,7 +616,7 @@ dhcp_add() { > > case $ra_management in > 0) > - # SLACC with DCHP for extended options > + # SLACC with DHCP for extended options > xappend "--dhcp-range=$nettag::,constructor:$ifname,ra-stateless,ra-names" > ;; > 2) > @@ -816,7 +816,7 @@ dnsmasq_start() > { > local cfg="$1" > local disabled user_dhcpscript > - local resolvfile resolvdir localuse=0 > + local resolvfile resolvdir leasedir localuse=0 > > config_get_bool disabled "$cfg" disabled 0 > [ "$disabled" -gt 0 ] && return 0 > @@ -994,7 +994,11 @@ dnsmasq_start() > fi > > config_get leasefile $cfg leasefile "/tmp/dhcp.leases" > - [ -n "$leasefile" ] && [ ! -e "$leasefile" ] && touch "$leasefile" > + [ -n "$leasefile" ] && { > + leasedir="$( dirname "$leasefile" )" && mkdir -p "$leasedir" > + [ ! -e "$leasefile" ] && touch "$leasefile" > + } > + > config_get_bool cachelocal "$cfg" cachelocal 1 > > config_get_bool noresolv "$cfg" noresolv 0 > @@ -1154,6 +1158,15 @@ dnsmasq_start() > procd_add_jail_mount $EXTRA_MOUNT $RFC6761FILE $TRUSTANCHORSFILE > procd_add_jail_mount $dnsmasqconffile $dnsmasqconfdir $resolvdir $user_dhcpscript > procd_add_jail_mount /etc/passwd /etc/group /etc/TZ /etc/hosts /etc/ethers > + > + [ -d "$leasedir" ] && { > + [ "$leasedir" = '/tmp' ] && { > + logger -t dnsmasq \ > + "consider using a more private directory for leasefile" \ > + "because jailing /tmp does not work: choose e.g. /tmp/dnsmasq/leasefile" > + } > + procd_add_jail_mount_rw $leasedir > + } > procd_add_jail_mount_rw /var/run/dnsmasq/ $leasefile > > procd_close_instance > -- > 2.30.2 >
On Mon, Oct 18, 2021 at 10:20:56AM +0100, Daniel Golle wrote: > On Mon, Oct 18, 2021 at 08:12:00AM +0000, Bastian Bittorf wrote: [...] > > This is v2 of the patch with a more correct description what is does. > > > > Ref: https://bugs.openwrt.org/index.php?do=details&task_id=4085 > > Suggested-by: Daniel Golle <daniel@makrotopia.org> > > I have neither Ack-ed nor suggested this change. I'am sorry about the confusion: I troubleshooted together with Daniel the issue and he proposed a change which i did unterstood in a wrong way, hence the situation. Sorry about that. Finally the underlying issue was even another thing, so I will bring up another patch, so please ignre the patch v2 too. bye, bastian
diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init index 3250b2179b..af2effdb26 100644 --- a/package/network/services/dnsmasq/files/dnsmasq.init +++ b/package/network/services/dnsmasq/files/dnsmasq.init @@ -616,7 +616,7 @@ dhcp_add() { case $ra_management in 0) - # SLACC with DCHP for extended options + # SLACC with DHCP for extended options xappend "--dhcp-range=$nettag::,constructor:$ifname,ra-stateless,ra-names" ;; 2) @@ -816,7 +816,7 @@ dnsmasq_start() { local cfg="$1" local disabled user_dhcpscript - local resolvfile resolvdir localuse=0 + local resolvfile resolvdir leasedir localuse=0 config_get_bool disabled "$cfg" disabled 0 [ "$disabled" -gt 0 ] && return 0 @@ -994,7 +994,11 @@ dnsmasq_start() fi config_get leasefile $cfg leasefile "/tmp/dhcp.leases" - [ -n "$leasefile" ] && [ ! -e "$leasefile" ] && touch "$leasefile" + [ -n "$leasefile" ] && { + leasedir="$( dirname "$leasefile" )" && mkdir -p "$leasedir" + [ ! -e "$leasefile" ] && touch "$leasefile" + } + config_get_bool cachelocal "$cfg" cachelocal 1 config_get_bool noresolv "$cfg" noresolv 0 @@ -1154,6 +1158,15 @@ dnsmasq_start() procd_add_jail_mount $EXTRA_MOUNT $RFC6761FILE $TRUSTANCHORSFILE procd_add_jail_mount $dnsmasqconffile $dnsmasqconfdir $resolvdir $user_dhcpscript procd_add_jail_mount /etc/passwd /etc/group /etc/TZ /etc/hosts /etc/ethers + + [ -d "$leasedir" ] && { + [ "$leasedir" = '/tmp' ] && { + logger -t dnsmasq \ + "consider using a more private directory for leasefile" \ + "because jailing /tmp does not work: choose e.g. /tmp/dnsmasq/leasefile" + } + procd_add_jail_mount_rw $leasedir + } procd_add_jail_mount_rw /var/run/dnsmasq/ $leasefile procd_close_instance
introduced with 44f694ba1bca1417d24e851c637c284f9f78c06d ("build: select procd-ujail if !SMALL_FLASH") dnsmasq fails to startup when the leasefile is configured to be in /tmp, which is just not suited for beeing a jail location. With this patch we explicit call jail_mount_rw() for the (now autocreated) leasedir and show a warning in syslog for the special case when leasefile is in directory /tmp without this patch, the syslog shows: Thu Oct 14 18:32:38 2021 user.err : jail: creat(/tmp/ujail-lhNbFK/tmp/dhcp.leases) failed: Read-only file system Thu Oct 14 18:32:38 2021 daemon.crit dnsmasq[1]: cannot open or create lease file /tmp/dhcp.leases: Read-only file system Thu Oct 14 18:32:38 2021 daemon.crit dnsmasq[1]: FAILED to start up This is v2 of the patch with a more correct description what is does. Ref: https://bugs.openwrt.org/index.php?do=details&task_id=4085 Suggested-by: Daniel Golle <daniel@makrotopia.org> Signed-off-by: Bastian Bittorf <bb@npl.de> --- .../services/dnsmasq/files/dnsmasq.init | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)