linux-kernel - Re: Build failure in -next due to 'hexagon: switch to RAW_COPY

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] [day] [month] [year] [list]

Date:   Mon, 24 Apr 2017 04:31:20 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-kernel@...r.kernel.org, Richard Kuo <rkuo@...eaurora.org>,
        Ley Foon Tan <lftan@...era.com>
Subject: Re: Build failure in -next due to 'hexagon: switch to RAW_COPY_USER'

On Sun, Apr 23, 2017 at 05:35:45PM -0700, Guenter Roeck wrote:
> hexagon:defconfig fails to build in -next with the following build error.
> 
> 
> In file included from include/linux/uaccess.h:13:0,
>                  from include/linux/poll.h:11,
>                  from include/linux/ring_buffer.h:7,
>                  from include/linux/trace_events.h:5,
>                  from include/trace/syscall.h:6,
>                  from include/linux/syscalls.h:82,
>                  from init/main.c:20:
> arch/hexagon/include/asm/uaccess.h: In function 'hexagon_strncpy_from_user':
> arch/hexagon/include/asm/uaccess.h:100:3: error: implicit declaration of function 'copy_from_user' [-Werror=implicit-function-declaration]
> In file included from include/linux/poll.h:11:0,
>                  from include/linux/ring_buffer.h:7,
>                  from include/linux/trace_events.h:5,
>                  from include/trace/syscall.h:6,
>                  from include/linux/syscalls.h:82,
>                  from init/main.c:20:
> include/linux/uaccess.h: At top level:
> include/linux/uaccess.h:145:1: error: conflicting types for 'copy_from_user'
> arch/hexagon/include/asm/uaccess.h:100:3: note: previous implicit declaration of 'copy_from_user' was here

Hmm...  FWIW, this
static inline long hexagon_strncpy_from_user(char *dst, const char __user *src,
                                             long n)
{
        long res = __strnlen_user(src, n);

        if (unlikely(!res))
                return -EFAULT;

        if (res > n) {
                copy_from_user(dst, src, n);
                return n;
        } else {
                copy_from_user(dst, src, res);
                return res-1;
        }
}
is bloody wrong.  Think what happens if in getname_flags() we have
__strnlen_user(filename, EMBEDDED_NAME_MAX) return 2.  We call
copy_from_user(kname, filename, 2) and return 1.  For caller it means
"one-character string, NUL-terminated", but there's no warranty that 
the second byte copied had been NUL.  And the destination was *NOT*
zero-filled, so there's nothing to guarantee us any zero bytes until
the end of allocated area.

If strncpy_from_user(dst, src, n) returns a number less than n, the result
must be NUL-terminated.  Looking through other architectures, nios2 appears
to be vulnerable to the same problem.  I hadn't finished looking
(xtensa, h8300 and score left to check), but the rest appears to be OK...