linux-kernel - Re: PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T

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]

Date:	Fri, 16 Mar 2012 00:52:50 -0700
From:	Paul Eggert <eggert@...ucla.edu>
To:	"H.J. Lu" <hjl.tools@...il.com>
CC:	LKML <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	GNU C Library <libc-alpha@...rceware.org>
Subject: Re: PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T_TYPE

On 03/15/2012 03:19 PM, H.J. Lu wrote:
> https://lkml.org/lkml/2012/2/8/408

That discussion does not seem to have considered the issue
of pointers, nor the issue of printf that Russ Allbery pointed out.
Here's an example from Kerrisk's "The Linux Programming Interface"
<http://man7.org/tlpi/code/online/dist/timers/t_clock_nanosleep.c.html>

            printf("... Remaining: %ld.%09ld",
                    (long) remain.tv_sec, remain.tv_nsec);

The proposed change breaks code like this.

>>  struct timespec t;
>>  long *p = &t->tv_nsec;
>> Such applications work fine now and conform to POSIX
> 
> GCC will complain about "incompatible pointer type".

True, and admittedly taking the address of tv_nsec is rarer than
printing it.  Still, it's just a warning and GCC goes ahead and builds
the program, and such warnings are often ignored.

> timespec is used in quite a few system calls. Checking all places
> which need to sign-extend is quite complex.

Many system calls copy timespec values from the kernel to the user;
these would be unaffected.  For syscalls that copy from the user
to the kernel, one could change glibc code like this:

  /* The Linux kernel can in some situations update the timeout value.          
     We do not want that so use a local variable.  */
  struct timespec tval;
  if (timeout != NULL)
    {
      tval = *timeout;
      timeout = &tval;
    }

(taken from glibc/sysdeps/unix/sysv/linux/pselect.c) to something like this:

  /* The Linux kernel can in some situations update the timeout value,
     or require a properly sign-extended timespec.  */
  struct timespec tval;
  if (timeout != NULL)
    {
      copy_timespec (&tval, timeout);
      timeout = &tval;
    }

where copy_timespec is an inline function that merely copies on existing
platforms, and also sign-extends tv_nsec on x32.  This doesn't appear complex,
though admittedly it does slow things down slightly on x32.

Another option, perhaps, would be to change the Linux kernel to
know about x32 binaries and to sign-extend tv_nsec inside the kernel,
when copying struct timespec objects from the user to the kernel.

Yet another option, I guess, would be to change POSIX so that tv_nsec could
be of type wider than 'long'.  However, this would seem to run afoul of
POSIX's intent, which is that system types like suseconds_t should
not be wider than 'long'; see
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
This constraint is to support uses like 'printf'.
Given the likelihood of breaking programs, it may be better simply
to conform to POSIX in this area, rather than change POSIX.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/