udhcpc: Wrong lease time is passed to the --script (PATCH)

[Markus Gothe]

Hi Karel,
nice patch.

There are two different standpoints here; one is to do like you
suggests. The other is to pass what the server actually provides to the
script.

My personal opinion is to fix-up this in the script itself if necessary,
else it will introduce an unexpected behavior.

BR,
Markus

On 2020-07-15 18:15, Karel Dolezal wrote:
> Hi,
>
> I've noticed udhcpc was not attempting to renew the address in time,
> in case the server offered
> only 30s lease time. The reason ended up being the "--script" which I
> use to install the address.
> It was still getting the 30s value, whereas udhcpc internally corrects
> the 30s value into 122s.
>
> Please see the proposed patch below.
>
> Best regards
> Karel
>
>
>
> Fixes DHCP lease time passed from udhcpc into the "--script"
>
>   For unusually short leases, udhcpc enforces a minimum lease time to avoid
>   excessive network traffic.
>
>   The original, unadjusted value, was however still being passed to the dhcp
>   event script, which was making it install the obtained IP address with a
>   lifetime shorter than the udhcpc's renewal period. As a result, the address
>   aged out and was removed without udhcpc attempting a renewal for some time.
>
>   This patch writes the corrected value back to the packet variable, from which
>   the script's environment is populated.
>
> Signed-off-by: Karel Dolezal <karel.dolezal at ui.com>
>
> --- a/networking/udhcp/dhcpc.c	2020-07-10 11:11:43.229706806 +0200
> +++ b/networking/udhcp/dhcpc.c	2020-07-10 11:17:30.366254882 +0200
> @@ -1867,16 +1867,21 @@ int udhcpc_main(int argc UNUSED_PARAM, c
>  					lease_seconds = 60 * 60;
>  				} else {
>  					/* it IS unaligned sometimes, don't "optimize" */
>  					move_from_unaligned32(lease_seconds, temp);
>  					lease_seconds = ntohl(lease_seconds);
>  					/* paranoia: must not be too small and not prone to overflows */
>  					/* timeout > 60 - ensures at least one unicast renew attempt */
> -					if (lease_seconds < 2 * 61)
> +					if (lease_seconds < 2 * 61) {
>  						lease_seconds = 2 * 61;
> +						/* The --script reads from the &packet variable itself. */
> +						/* Write the adjusted value back there, to give script */
> +						/* the same lease time as we'll use internally. */
> +						move_to_unaligned32(temp, htonl(lease_seconds));
> +					}
>  					//if (lease_seconds > 0x7fffffff)
>  					//	lease_seconds = 0x7fffffff;
>  					//^^^not necessary since "timeout = lease_seconds / 2"
>  					//does not overflow even for 0xffffffff.
>  				}
>  #if ENABLE_FEATURE_UDHCPC_ARPING
>  				if (opt & OPT_a) {
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox