linux-kernel - Re: [GIT pull] timer updates for 4.19

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: <CAK8P3a28q8PrKGFzp7fS98sXP0ZH3QS8s9gEuZo2DYWQ2CfzTQ@mail.gmail.com>
Date:   Mon, 27 Aug 2018 10:24:15 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>, Christoph Hellwig <hch@....de>
Subject: Re: [GIT pull] timer updates for 4.19

On Sun, Aug 26, 2018 at 7:13 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Sun, Aug 26, 2018 at 2:39 AM Thomas Gleixner <tglx@...utronix.de> wrote:
> >
> > New defines for the compat time* types so they can be shared between 32bit
> > and 64bit builds. Not used yet, but merging them now allows the actual
> > conversions to be merged through different maintainer trees without
> > dependencies
>
> So I pulled this, then looked at it a bit more, and went "that's stupid".
>
> Why introduce a completely useless new name for something we already have?
>
> Why can't the 32-bit code just use the "compat" names instead? Even
> for 32-bit systems, it's about compatibility with the old world order.
>
> So what's the advantage of calling it "old" over just calling it "compat"?
>
> I do not for a *second* believe that "compat" is somehow confusing to
> 32-bit architectures. They all have the new 64-bit time code already,
> it's not like there is any confusion what-so-ever about what is going
> on here.

I'd argue that Christoph was demonstrably confused by the usage on
32-bit architectures, and then argued strongly against it [1].

I postponed the series for the remaining syscall conversion based
on his feedback, but didn't manage to get it all ready in time.
If you think we should go back to the previous version, that would
also work, though I still think the new version is somewhat clearer
now, having implemented both of them.

The difference is best demonstrated with a syscall that takes
both time_t and other incompatible arguments.

a) old method: share the compat syscall between 64-bit and 32-bit
architectures, three entry points for a few syscalls:

/* native syscall, always 64-bit time_t */
SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
   siginfo_t __user *, uinfo, const struct __kernel_timespec __user *, uts,
   size_t, sigsetsize)

/* native/compat syscall, always 32-bit time_t, ugly and confusing */
#ifdef CONFIG_COMPAT_32BIT_TIME
#ifndef CONFIG_COMPAT
#define compat_siginfo siginfo
#define compat_sigset_t sigset_t
#define copy_siginfo_to_user32 copy_siginfo_to_user
static inline int get_compat_sigset(sigset_t *set, const sigset_t
__user *compat)
{
 if (copy_from_user(set, compat, sizeof *set))
       return -EFAULT;
 return 0;
}
#endif
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait,
   const compat_sigset_t __user *, uthese,
   compat_siginfo_t __user *, uinfo,
   const struct compat_timespec __user *, uts,
   compat_size_t, sigsetsize)
#endif

/* compat syscall for 32-bit sigset_t with 64-bit time_t */
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait_time64,
   const compat_sigset_t __user *, uthese,
   compat_siginfo_t __user *, uinfo,
   const struct __kernel_timespec __user *, uts,
   compat_size_t, sigsetsize)
#endif


b) new method: 32-bit old_time32_t, never use compat identifiers
  on 32-bit architectures, four entry points for a few syscalls

/* native sigset, 32-bit time */
#ifdef CONFIG_COMPAT_32BIT_TIME
SYSCALL_DEFINE4(rt_sigtimedwait_time32, const sigset_t __user *, uthese,
   siginfo_t __user *, uinfo, const struct old_timespec32 __user *, uts,
   size_t, sigsetsize)
#endif

/* native sigset, 64-bit time */
SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
   siginfo_t __user *, uinfo, const struct __kernel_timespec __user *, uts,
   size_t, sigsetsize)

#ifdef CONFIG_COMPAT
/* compat sigset, 32-bit time */
#ifdef CONFIG_COMPAT_32BIT_TIME
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait_time32,
   const compat_sigset_t __user *, uthese,
   compat_siginfo_t __user *, uinfo,
   const struct old_timespec32 __user *, uts,
   compat_size_t, sigsetsize)
#endif

/* compat sigset, 64-bit time */
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait,
   const compat_sigset_t __user *, uthese,
   compat_siginfo_t __user *, uinfo,
   const struct __kernel_timespec __user *, uts,
   compat_size_t, sigsetsize)
#endif

Only a handful of syscalls actually require that complexity, most of them
need only two entry points either way (32-bit time, and 64-bit time), the
question is what we call them. I have a mild preference for the second
version after the discussion with Christoph, but I'm also fine with going back
to the orignal plan if you like that better after all.

       Arnd

[1] https://lore.kernel.org/lkml/20180705213604.18883-2-deepa.kernel@gmail.com/