Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [RFC] y2038: HCI_TIME_STAMP with time64
@ 2020-01-10 10:11 Arnd Bergmann
  2020-01-10 15:44 ` Marcel Holtmann
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2020-01-10 10:11 UTC (permalink / raw)
  To: Bluez mailing list
  Cc: y2038 Mailman List, Johan Hedberg, Marcel Holtmann,
	Linux Kernel Mailing List, Deepa Dinamani, Rich Felker

I noticed earlier this week that the HCI_CMSG_TSTAMP/HCI_TIME_STAMP
interface has no time64 equivalent, as we apparently missed that when
converting the normal socket timestamps to support both time32 and time64
variants of the sockopt and cmsg data.

The interface was originally added back in 2002 by Maksim Krasnyanskiy
when bluetooth support first became non-experimental.

When using HCI_TIME_STAMP on a 32-bit system with a time64
libc, users will interpret the { s32 tv_sec; s32 tv_usec } layout of
the kernel as { s64 tv_sec; ... }, which puts complete garbage
into the timestamp regardless of whether this code runs before or
after y2038. From looking at codesearch.debian.org, I found two
users of this: libpcap and hcidump. There are probably others that
are not part of Debian.

Fixing this the same was as normal socket timestamps is not possible
because include/net/bluetooth/hci.h is not an exported UAPI header.
This means any changes to it for defining HCI_TIME_STAMP conditionally
would be ignored by applications that use a different copy of the
header.

I can see three possible ways forward:

1. move include/net/bluetooth/hci.h to include/uapi/, add a conditional
   definition of HCI_TIME_STAMP and make the kernel code support
   both formats. Then change applications to rely on that version of
   header file to get the correct definition but not change application code.

2. Leave the kernel completely unchanged and modify only the users
    to not expect the output to be a 'struct timeval' but interpret as
    as { uint32_t tv_sec; int32_t tv_usec; } structure on 32-bit architectures,
    which will work until the unsigned time overflows 86 years from now
    in 2106 (same as the libpcap on-disk format).

3. Add support for the normal SO_TIMESTAMPNS_NEW sockopt in
   HCI, providing timestamps in the unambiguous { long long tv_sec;
   long long tv_nsec; } format to user space, and change applications
   to use that if supported by the kernel.

      Arnd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] y2038: HCI_TIME_STAMP with time64
  2020-01-10 10:11 [RFC] y2038: HCI_TIME_STAMP with time64 Arnd Bergmann
@ 2020-01-10 15:44 ` Marcel Holtmann
  2020-01-10 16:08   ` Arnd Bergmann
  0 siblings, 1 reply; 4+ messages in thread
From: Marcel Holtmann @ 2020-01-10 15:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Bluez mailing list, y2038 Mailman List, Johan Hedberg,
	Linux Kernel Mailing List, Deepa Dinamani, Rich Felker

Hi Arnd,

> I noticed earlier this week that the HCI_CMSG_TSTAMP/HCI_TIME_STAMP
> interface has no time64 equivalent, as we apparently missed that when
> converting the normal socket timestamps to support both time32 and time64
> variants of the sockopt and cmsg data.
> 
> The interface was originally added back in 2002 by Maksim Krasnyanskiy
> when bluetooth support first became non-experimental.
> 
> When using HCI_TIME_STAMP on a 32-bit system with a time64
> libc, users will interpret the { s32 tv_sec; s32 tv_usec } layout of
> the kernel as { s64 tv_sec; ... }, which puts complete garbage
> into the timestamp regardless of whether this code runs before or
> after y2038. From looking at codesearch.debian.org, I found two
> users of this: libpcap and hcidump. There are probably others that
> are not part of Debian.
> 
> Fixing this the same was as normal socket timestamps is not possible
> because include/net/bluetooth/hci.h is not an exported UAPI header.
> This means any changes to it for defining HCI_TIME_STAMP conditionally
> would be ignored by applications that use a different copy of the
> header.
> 
> I can see three possible ways forward:
> 
> 1. move include/net/bluetooth/hci.h to include/uapi/, add a conditional
>   definition of HCI_TIME_STAMP and make the kernel code support
>   both formats. Then change applications to rely on that version of
>   header file to get the correct definition but not change application code.
> 
> 2. Leave the kernel completely unchanged and modify only the users
>    to not expect the output to be a 'struct timeval' but interpret as
>    as { uint32_t tv_sec; int32_t tv_usec; } structure on 32-bit architectures,
>    which will work until the unsigned time overflows 86 years from now
>    in 2106 (same as the libpcap on-disk format).
> 
> 3. Add support for the normal SO_TIMESTAMPNS_NEW sockopt in
>   HCI, providing timestamps in the unambiguous { long long tv_sec;
>   long long tv_nsec; } format to user space, and change applications
>   to use that if supported by the kernel.

I have added SO_TIMESTAMP* to every Bluetooth socket a while back. And that should be used by the majority of the tools. One exception might by hcidump which has been replaced by btmon already anyway.

So I would not bother with HCI_TIME_STAMP fixing. We can do 2) if someone really still wants to use that socket option. However I am under the impression that 3) should be already possible.

Regards

Marcel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] y2038: HCI_TIME_STAMP with time64
  2020-01-10 15:44 ` Marcel Holtmann
@ 2020-01-10 16:08   ` Arnd Bergmann
  2020-01-10 17:52     ` Marcel Holtmann
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2020-01-10 16:08 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Bluez mailing list, y2038 Mailman List, Johan Hedberg,
	Linux Kernel Mailing List, Deepa Dinamani, Rich Felker,
	Guy Harris

On Fri, Jan 10, 2020 at 4:44 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> > I noticed earlier this week that the HCI_CMSG_TSTAMP/HCI_TIME_STAMP
> > interface has no time64 equivalent, as we apparently missed that when
> > converting the normal socket timestamps to support both time32 and time64
> > variants of the sockopt and cmsg data.
...
> > When using HCI_TIME_STAMP on a 32-bit system with a time64
> > libc, users will interpret the { s32 tv_sec; s32 tv_usec } layout of
> > the kernel as { s64 tv_sec; ... }, which puts complete garbage
> > into the timestamp regardless of whether this code runs before or
> > after y2038. From looking at codesearch.debian.org, I found two
> > users of this: libpcap and hcidump. There are probably others that
> > are not part of Debian.
...
> > 3. Add support for the normal SO_TIMESTAMPNS_NEW sockopt in
> >   HCI, providing timestamps in the unambiguous { long long tv_sec;
> >   long long tv_nsec; } format to user space, and change applications
> >   to use that if supported by the kernel.
>
> I have added SO_TIMESTAMP* to every Bluetooth socket a while back. And that should be used by the majority of the tools. One exception might by hcidump which has been replaced by btmon already anyway.
>
> So I would not bother with HCI_TIME_STAMP fixing. We can do 2) if someone really still wants to use that socket option. However I am under the impression that 3) should be already possible.

Ok, excellent, I had not realized this works already.

I have now also checked
https://github.com/the-tcpdump-group/libpcap/blob/master/pcap-bt-monitor-linux.c
which uses SO_TIMESTAMP and then should work. I guess this is similar
to what btmon does.

For libpcap that leaves
https://github.com/the-tcpdump-group/libpcap/blob/master/pcap-bt-linux.c#L358

which needs a fairly simply fix on 32-bit architectures to copy the
two 32-bit fields
into the longer pkth.ts fields individually rather than using a memcpy.
I've added Guy Harris to Cc, he seems to be the maintainer for this file
according to the git history.

The same change is needed for
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/hcidump.c#n233
if there are any remaining users. I can send you a patch if you want.

        Arnd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] y2038: HCI_TIME_STAMP with time64
  2020-01-10 16:08   ` Arnd Bergmann
@ 2020-01-10 17:52     ` Marcel Holtmann
  0 siblings, 0 replies; 4+ messages in thread
From: Marcel Holtmann @ 2020-01-10 17:52 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Bluez mailing list, y2038 Mailman List, Johan Hedberg,
	Linux Kernel Mailing List, Deepa Dinamani, Rich Felker,
	Guy Harris

Hi Arnd,

>>> I noticed earlier this week that the HCI_CMSG_TSTAMP/HCI_TIME_STAMP
>>> interface has no time64 equivalent, as we apparently missed that when
>>> converting the normal socket timestamps to support both time32 and time64
>>> variants of the sockopt and cmsg data.
> ...
>>> When using HCI_TIME_STAMP on a 32-bit system with a time64
>>> libc, users will interpret the { s32 tv_sec; s32 tv_usec } layout of
>>> the kernel as { s64 tv_sec; ... }, which puts complete garbage
>>> into the timestamp regardless of whether this code runs before or
>>> after y2038. From looking at codesearch.debian.org, I found two
>>> users of this: libpcap and hcidump. There are probably others that
>>> are not part of Debian.
> ...
>>> 3. Add support for the normal SO_TIMESTAMPNS_NEW sockopt in
>>>  HCI, providing timestamps in the unambiguous { long long tv_sec;
>>>  long long tv_nsec; } format to user space, and change applications
>>>  to use that if supported by the kernel.
>> 
>> I have added SO_TIMESTAMP* to every Bluetooth socket a while back. And that should be used by the majority of the tools. One exception might by hcidump which has been replaced by btmon already anyway.
>> 
>> So I would not bother with HCI_TIME_STAMP fixing. We can do 2) if someone really still wants to use that socket option. However I am under the impression that 3) should be already possible.
> 
> Ok, excellent, I had not realized this works already.
> 
> I have now also checked
> https://github.com/the-tcpdump-group/libpcap/blob/master/pcap-bt-monitor-linux.c
> which uses SO_TIMESTAMP and then should work. I guess this is similar
> to what btmon does.
> 
> For libpcap that leaves
> https://github.com/the-tcpdump-group/libpcap/blob/master/pcap-bt-linux.c#L358
> 
> which needs a fairly simply fix on 32-bit architectures to copy the
> two 32-bit fields
> into the longer pkth.ts fields individually rather than using a memcpy.
> I've added Guy Harris to Cc, he seems to be the maintainer for this file
> according to the git history.
> 
> The same change is needed for
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/hcidump.c#n233
> if there are any remaining users. I can send you a patch if you want.

please send a patch for hcidump if you have the time.

Regards

Marcel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 10:11 [RFC] y2038: HCI_TIME_STAMP with time64 Arnd Bergmann
2020-01-10 15:44 ` Marcel Holtmann
2020-01-10 16:08   ` Arnd Bergmann
2020-01-10 17:52     ` Marcel Holtmann

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git