Re: Fourth draft of the Y2038 design document

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday, June 29, 2016 11:58:38 AM CEST Albert ARIBAUD wrote:

> > == IOCTLs and Y2038 ==
> > 
> >  Some Linux IOCTLs may be Y2038-unsafe, or use types defined by glibc
> >  that do not match the kernel internal types. Known important cases are:
> > 
> >  - An ioctl command number is defined using the _IOR/_IOW/_IORW macros
> >    by the kernel with a structure whose size changes based on glibc's
> >    time_t. The kernel can handle these transparently by implementing
> >    handlers for both command numbers with the correct structure format.
> > 
> >  - The binary ABI changes based on the glibc time_t type, but the
> >    command number does not change. In this case, the kernel header files
> >    defining the data structure will check the "__USE_TIME_BITS64"
> >    macro [do we need a new macro for the kernel headers?] to provide
> >    a different command number for the new data structure layout. glibc
> >    will define this macro at an appropriate location [where?] to make
> >    it visible before including kernel header files.
> > 
> >  - An ioctl command passes time information in a structure that is not
> >    based on time_t but another integer type that does not get changed.
> >    The kernel header files will provide both a new structure layout
> >    and command number when "__USE_TIME_BITS64" is set.
> > 
> > [I can find examples for ioctl commands in each of those
> > categories if needed.]
> 
> Please do -- I understand the first two cases, but I am not sure I
> understand the third one.


a) include/linux/ppdev.h:

/* Set and get port timeout (struct timeval's) */
#define PPGETTIME       _IOR(PP_IOCTL, 0x95, struct timeval)
#define PPSETTIME       _IOW(PP_IOCTL, 0x96, struct timeval)

b) include/linux/lp.h: (this passes a timeval as well)
#define LPSETTIMEOUT 0x060f /* set parport timeout */

c) Actually I could not find a real case any more, it's possible they
are all gone. Two similar cases though:

linux/raid/md_u.h: (this one was "fixed" by ensuring the user space
side can deal with 32-bit time stamps)

typedef struct mdu_array_info_s {
       ...
        unsigned int utime;     /*  0 Superblock update time                  */
        ...
} mdu_array_info_t;
#define GET_ARRAY_INFO          _IOR (MD_MAJOR, 0x11, mdu_array_info_t)


drivers/staging/ft1000/ft1000-usb/ft1000_ioctl.h: (this file was
removed since, as the hardware is now obsolete)

struct IOCTL_GET_DSP_STAT {
       unsigned char DspVer[DSPVERSZ];        /* DSP version number */
       unsigned char HwSerNum[HWSERNUMSZ];    /* Hardware Serial Number */
       unsigned char Sku[SKUSZ];              /* SKU */
       unsigned char eui64[EUISZ];            /* EUI64 */
       unsigned short ConStat;                /* Connection Status */

       unsigned long nTxPkts;                /* Number of packets transmitted
                                              * from host to dsp
                                              */
       unsigned long nRxPkts;                /* Number of packets received from
                                              * dsp to host
                                              */
       unsigned long nTxBytes;               /* Number of bytes transmitted
                                              * from host to dsp
                                              */
       unsigned long nRxBytes;               /* Number of bytes received from
                                              * dsp to host
                                              */
       unsigned long ConTm;                  /* Current session connection time
                                              * in seconds
                                              */
      ....
} __packed;

#define IOCTL_FT1000_GET_DSP_STAT _IOR(FT1000_MAGIC_CODE,              \
                                      IOCTL_GET_DSP_STAT_CMD,          \
                                      struct IOCTL_GET_DSP_STAT)

> > == Socket options ==
> > 
> > Like ioctl(), setsockopt()/getsockopt() has a few interfaces that are
> > passing time data:
> > 
> > SO_TIMESTAMP/SO_TIMESTAMPNS/SO_TIMESTAMPING: These enable the
> > timestamping infrastructure for a socket, which will consecutively
> > return data to user space using "cmsg" data on the socket. The kernel
> > does not know the layout of 'struct timespec' and 'struct timeval'
> > when filling the cmsg data, so we need to define new binary values
> > for the three flags, which then get used if __USE_TIME_BITS64
> > is set.
> 
> IOW, introduce three new optname values (say 51, 52, and 53 as on my
> machine, /usr/include/asm-generic/socket.h stops at 50 right now) that
> would behave similar to options 29, 35 and 37 but would use Y2038-safe
> timestamps; and define option names SO_TIMESTAMP, SO_TIMESTAMPNS and
> SO_TIMESTAMPING to be either 29, 35 and 37 (the Y2038-unsafe options) or
> 51, 52 and 53 (the Y2038-safe ones) depending on __USE_TIME_BITS64.
> Right?

Correct.
 
> > SO_RCVTIMEO/SO_SNTTIMEO: These pass a 'struct timeval' and a length.
> > Assuming that the 'optlen' argument of the setsockopt syscall always
> > matches the size of 'struct timeval', the kernel will be able to
> > access the data in the same format that was passed by glibc.
> > [alternatively,
> > we could handle this the same way as SO_TIMESTAMP*, using new numbers
> > for the flags].
> 
> I would prefer new numbers, that's more explicit and slightly safer,
> as if we rely on optlen, Murphy's Law will see to it that in practice
> it will have the wrong value.

Fair enough.

	Arnd