Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Jul 2023 09:34:42 -0700
From:      Cy Schubert <Cy.Schubert@cschubert.com>
To:        Eric van Gyzen <vangyzen@FreeBSD.org>
Cc:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   Re: git: c210cac00f25 - main - dhclient: fix time parsing for  leases expiring after 2038
Message-ID:  <20230710163442.8CE952D@slippy.cwsent.com>
In-Reply-To: <202307101600.36AG0xNe015335@gitrepo.freebsd.org>
References:  <202307101600.36AG0xNe015335@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
In message <202307101600.36AG0xNe015335@gitrepo.freebsd.org>, Eric van 
Gyzen wr
ites:
> The branch main has been updated by vangyzen:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=c210cac00f2584f031b56b4cdd2e801d
> bb9e1348
>
> commit c210cac00f2584f031b56b4cdd2e801dbb9e1348
> Author:     Alex Bahm <alexander.bahm@dell.com>
> AuthorDate: 2023-07-10 15:56:05 +0000
> Commit:     Eric van Gyzen <vangyzen@FreeBSD.org>
> CommitDate: 2023-07-10 16:00:34 +0000
>
>     dhclient: fix time parsing for leases expiring after 2038
>     
>     Convert lease parsing to timegm to calculate timestamp. For reference, wh
> en
>     writing the lease, we use gmtime to convert the timestamp to struct tm.
>     
>     Reviewed By:    markj, vangyzen
>     MFC after:      2 weeks
>     Sponsored by:   Dell EMC Isilon
>     Differential Revision:  https://reviews.freebsd.org/D40760
> ---
>  sbin/dhclient/parse.c                      | 28 ++-----------------
>  sbin/dhclient/tests/Makefile               |  3 +-
>  sbin/dhclient/tests/fake.c                 | 15 ++++++++++
>  sbin/dhclient/tests/option-domain-search.c | 45 ++++++++++++++++++++++++++++
> ++
>  4 files changed, 64 insertions(+), 27 deletions(-)
>
> diff --git a/sbin/dhclient/parse.c b/sbin/dhclient/parse.c
> index 4018f70f3a5d..70e2013d0813 100644
> --- a/sbin/dhclient/parse.c
> +++ b/sbin/dhclient/parse.c
> @@ -444,9 +444,7 @@ convert_num(unsigned char *buf, char *str, unsigned base,
>  int size)
>  time_t
>  parse_date(FILE *cfile)
>  {
> -	static int months[11] = { 31, 59, 90, 120, 151, 181,
> -	    212, 243, 273, 304, 334 };
> -	int guess, token;
> +	int token;
>  	struct tm tm;
>  	char *val;
>  
> @@ -570,27 +568,5 @@ parse_date(FILE *cfile)
>  		return (0);
>  	}
>  
> -	/* Guess the time value... */
> -	guess = ((((((365 * (tm.tm_year - 70) +	/* Days in years since '70 */
> -		    (tm.tm_year - 69) / 4 +	/* Leap days since '70 */
> -		    (tm.tm_mon			/* Days in months this year */
> -		    ? months[tm.tm_mon - 1]
> -		    : 0) +
> -		    (tm.tm_mon > 1 &&		/* Leap day this year */
> -		    !((tm.tm_year - 72) & 3)) +
> -		    tm.tm_mday - 1) * 24) +	/* Day of month */
> -		    tm.tm_hour) * 60) +
> -		    tm.tm_min) * 60) + tm.tm_sec;
> -
> -	/*
> -	 * This guess could be wrong because of leap seconds or other
> -	 * weirdness we don't know about that the system does.   For
> -	 * now, we're just going to accept the guess, but at some point
> -	 * it might be nice to do a successive approximation here to get
> -	 * an exact value.   Even if the error is small, if the server
> -	 * is restarted frequently (and thus the lease database is
> -	 * reread), the error could accumulate into something
> -	 * significant.
> -	 */
> -	return (guess);
> +	return (timegm(&tm));
>  }
> diff --git a/sbin/dhclient/tests/Makefile b/sbin/dhclient/tests/Makefile
> index ce4c7acb822e..790d3dbcccce 100644
> --- a/sbin/dhclient/tests/Makefile
> +++ b/sbin/dhclient/tests/Makefile
> @@ -6,7 +6,8 @@ ATF_TESTS_SH=	pcp
>  
>  PLAIN_TESTS_C=				option-domain-search_test
>  SRCS.option-domain-search_test=		alloc.c convert.c hash.c option
> s.c \
> -					tables.c fake.c option-domain-search.c
> +					tables.c parse.c conflex.c tree.c fake.
> c \
> +					option-domain-search.c
>  CFLAGS.option-domain-search_test+=	-I${.CURDIR:H}
>  LIBADD.option-domain-search_test=	util
>  
> diff --git a/sbin/dhclient/tests/fake.c b/sbin/dhclient/tests/fake.c
> index 6a170953beb0..17b721527f04 100644
> --- a/sbin/dhclient/tests/fake.c
> +++ b/sbin/dhclient/tests/fake.c
> @@ -7,6 +7,7 @@
>  #include "dhcpd.h"
>  
>  extern jmp_buf env;
> +int warnings_occurred;
>  
>  void
>  error(const char *fmt, ...)
> @@ -52,6 +53,20 @@ note(const char *fmt, ...)
>  	return ret;
>  }
>  
> +int
> +parse_warn(const char *fmt, ...)
> +{
> +	int ret;
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	ret = vfprintf(stderr, fmt, ap);
> +	va_end(ap);
> +	fprintf(stderr, "\n");
> +
> +	return ret;
> +}
> +
>  void
>  bootp(struct packet *packet)
>  {
> diff --git a/sbin/dhclient/tests/option-domain-search.c b/sbin/dhclient/tests
> /option-domain-search.c
> index b79f9a560137..a3517c9c1dc1 100644
> --- a/sbin/dhclient/tests/option-domain-search.c
> +++ b/sbin/dhclient/tests/option-domain-search.c
> @@ -303,6 +303,49 @@ multiple_domains_valid()
>  	free(option->data);
>  }
>  
> +static
> +void
> +parse_date_helper(const char *string, time_t timestamp)
> +{
> +	int ret = 0;
> +	FILE *file = NULL;
> +	time_t ts;
> +
> +	file = fopen("/tmp/dhclient.test", "w");
> +	if (!file)
> +		abort();
> +
> +	ret = fwrite(string, strlen(string), 1, file);
> +	if (ret <= 0)
> +		abort();
> +
> +	fclose(file);
> +
> +	file = fopen("/tmp/dhclient.test", "r");
> +	if (!file)
> +		abort();
> +
> +	new_parse("test");
> +	ts = parse_date(file);
> +	if (ts != timestamp)
> +		abort();
> +
> +	fclose(file);
> +}
> +
> +void
> +parse_date_valid(void)
> +{
> +	int ret;
> +
> +	ret = setjmp(env);
> +	if (ret != 0)
> +		abort();
> +
> +	parse_date_helper(" 2 2024/7/2 20:25:50;\n", 1719951950);
> +	parse_date_helper(" 1 2091/7/2 20:25:50;\n", 3834246350);
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -324,5 +367,7 @@ main(int argc, char *argv[])
>  
>  	multiple_domains_valid();
>  
> +	parse_date_valid();
> +
>  	return (0);
>  }
>

This fails to build on i386.

===> libexec/bootpd/tools/bootptest (all)
--- all_subdir_sbin ---
/opt/src/git-src/sbin/dhclient/tests/option-domain-search.c:346:47: error: 
implicit conversion from 'long long' to 'time_t' (aka 'int') changes value 
from 3834246350 to -460720946 [-Werror,-Wconstant-conversion]
        parse_date_helper(" 1 2091/7/2 20:25:50;\n", 3834246350);
        ~~~~~~~~~~~~~~~~~                            ^~~~~~~~~~
1 error generated.
        4.16 real         0.37 user         0.36 sys

make[1]: stopped in /opt/src/git-src

make: stopped in /opt/src/git-src
_buildsys 58228 - - i386 build failed, continuing
_buildsys 58233 - - i386 failed: build failure
_buildsys 58238 - - exiting, RC=0: /usr/local/bin/_buildsys -x -uz -Nb 
-Ai386 -j8
Finished Mon Jul 10 09:31:24 PDT 2023, RC=0


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

			e^(i*pi)+1=0






Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20230710163442.8CE952D>