linux-kernel - Re: [PATCH] hcidump: add support for time64 based libc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <25105A85-A3EE-4E6A-AAF5-D54FCF9E6ACB@alum.mit.edu>
Date:   Fri, 10 Jan 2020 15:30:16 -0800
From:   Guy Harris <guy@...m.mit.edu>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Bluez mailing list <linux-bluetooth@...r.kernel.org>,
        y2038 Mailman List <y2038@...ts.linaro.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Rich Felker <dalias@...c.org>
Subject: Re: [PATCH] hcidump: add support for time64 based libc

On Jan 10, 2020, at 12:49 PM, Arnd Bergmann <arnd@...db.de> wrote:

> diff --git a/monitor/hcidump.c b/monitor/hcidump.c
> index 8b6f846d3..6d2330287 100644
> --- a/monitor/hcidump.c
> +++ b/monitor/hcidump.c
> @@ -107,6 +107,36 @@ static int open_hci_dev(uint16_t index)
> 	return fd;
> }
> 
> +static struct timeval hci_tstamp_read(void *data)
> +{
> +	struct timeval tv;
> +
> +	/*
> +	 * On 64-bit architectures, the data matches the timeval
> +	 * format. Note that on sparc64 this is different from
> +	 * all others.
> +	 */
> +	if (sizeof(long) == 8) {
> +		memcpy(&tv, data, sizeof(tv));
> +	}
> +
> +	/*
> +	 * On 32-bit architectures, the timeval definition may
> +	 * use 32-bit or 64-bit members depending on the C
> +	 * library and architecture.
> +	 * The cmsg data however always contains a pair of
> +	 * 32-bit values. Interpret as unsigned to make it work
> +	 * past y2038.
> +	 */
> +	if (sizeof(long) == 4) {
> +		unsigned int *stamp = data;
> +		tv.tv_sec = stamp[0];
> +		tv.tv_usec = stamp[1];
> +	}
> +
> +	return tv;
> +}

Should it be something more like

	if (sizeof(long) == 8) {
		/*
		 * On 64-bit architectures, the data matches the timeval
		 * format. Note that on sparc64 this is different from
		 * all others.
		 */
		memcpy(&tv, data, sizeof(tv));
	} else if (sizeof(long) == 4) {
		/*
		 * On 32-bit architectures, the timeval definition may
		 * use 32-bit or 64-bit members depending on the C
		 * library and architecture.
		 * The cmsg data however always contains a pair of
		 * 32-bit values. Interpret as unsigned to make it work
		 * past y2038.
		 */
		unsigned int *stamp = data;
		tv.tv_sec = stamp[0];
		tv.tv_usec = stamp[1];
	} else {
		abort();	/* or some other "sorry, we're not ready for 128-bit or weird architectures yet" failure */
	}

	return tv;

> static void device_callback(int fd, uint32_t events, void *user_data)
> {
> 	struct hcidump_data *data = user_data;
> @@ -150,7 +180,7 @@ static void device_callback(int fd, uint32_t events, void *user_data)
> 				memcpy(&dir, CMSG_DATA(cmsg), sizeof(dir));
> 				break;
> 			case HCI_CMSG_TSTAMP:
> -				memcpy(&ctv, CMSG_DATA(cmsg), sizeof(ctv));
> +				ctv = hci_tstamp_read(CMSG_DATA(cmsg));
> 				tv = &ctv;
> 				break;
> 			}

And libpcap's Linux BT code should do the same thing, changing its memcpy() call?

If you want, you can submit a pull request, or I can make the change.