On Thu, Feb 23, 2017 at 12:11 AM, Joseph Myers joseph@codesourcery.com wrote:
On Wed, 22 Feb 2017, Arnd Bergmann wrote:
In "Y2038-compatible struct timespec", replace "microseconds" with "nanoseconds. Also, it's worth pointing out the known problems with the padding:
- on big-endian systems, any code doing a variation of "struct timespec ts = { seconds, nanos };" is broken because it assigns the nanoseconds to the wrong struct member. The example code is nonconforming as neither POSIX nor C11 require a particular order of the struct members, but I could also easily find examples of existing programs doing this. Note that NetBSD, OpenBSD and Windows have no padding but do use 64-bit time_t.
- If the padding is uninitialized, we have to explicitly zero it before calling a kernel function that assumes the 64-bit layout. This can be done in glibc before calling into the kernel, or at the kernel entry (where my last patch set does it), but it is awkward either way.
Unfortunately, there doesn't seem to be a good solution here for either of the two problems. Maybe someone else has more ideas. Using a pointer type for the padding would at least cause a compile-time warning for broken code, other solutions might require GCC extensions or break C11 in another way.
You could declare the padding as an unnamed bit-field "int : 32;" to avoid it affecting initialization. But that complicated zeroing it (in cases where you're zeroing the original struct - when it's a read/write parameter passed to the kernel - rather than a copy), as you can no longer refer to it by name to assign zero to it; maybe you'd need to arrange for it to be named inside glibc but unnamed for user code.
I had thought of that as well, but wasn't sure if we could rely on bitfields to do the right thing across C standard versions and non-gcc compilers. I experimentally found that gcc-4.9 and higher accept that version without a -Wpedantic warning when using --std=c99 or --std=c11, but they warn about it with --std=c89 or --std=gnu89. Older gcc versions as far back as gcc-3.4 (the oldest I could try) always warn about anonymous bitfields with -pedantic, but I could still silence that warning using the __extension__ keyword. I think that's good enough.
With that problem out of the way, we'd still need to figure out where to clear the padding on the way from libc into the kernel. I think the tradeoff is:
Pro glibc: - IIRC already have code to do this for x32, and could do it the same way on all 32-bit architectures. (I could not find where this is done though, maybe I dreamed it up?) - We don't slow down the normal 64-bit kernel syscall path for every get_user_timespec64() by checking whether we are called from a 32-bit or 64-bit task to decide whether we should clear the upper bits or not.
Pro kernel: - No need to copy the data structure in user space to avoid writing into read-only variables - Can handle both ioctl and syscall path with the same helper function, no need for random ioctl users to clear the upper bits
Arnd