From patchwork Tue Feb 27 15:31:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lance Richardson X-Patchwork-Id: 10245677 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 087F560211 for ; Tue, 27 Feb 2018 15:31:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ED73328731 for ; Tue, 27 Feb 2018 15:31:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E20532873E; Tue, 27 Feb 2018 15:31:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C53A628731 for ; Tue, 27 Feb 2018 15:31:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753514AbeB0Pbs (ORCPT ); Tue, 27 Feb 2018 10:31:48 -0500 Received: from mail-qk0-f196.google.com ([209.85.220.196]:41104 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752798AbeB0Pbr (ORCPT ); Tue, 27 Feb 2018 10:31:47 -0500 Received: by mail-qk0-f196.google.com with SMTP id w142so12185967qkb.8 for ; Tue, 27 Feb 2018 07:31:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=Nnqng4e3wmxG5DwoLBgFI4EQ9L7xakS5OJFwO/XXGfc=; b=FN8w7/fB6qV3ymIVEuYuKD3ITqSwNqHYmSYHuy0xISMstXlf7ny85wi85sJPrdMW0Y 8TiInvySauQfT05Le4fNuV1XOG+OG6pldX4u1VxgilDnJrctPNJifntBLDg6xTzx7exJ ObGZ11rEb74H04KPt9+DozV2jAbodirS8iV1uzyrsd2kv4W2HfhC3TCy3fDW8GjOh9Sh lLFszi46c+/PFUkV9oLEiit5wQ7q+L7EGodf9ecJcpCfYQp/ghXvR5GLqr+YNaCDQOtH WuP8gaLOrlKdSW84MpgZcUJPYoYrQrcpKtrB3+yD8tbABKOodk6BNt5vCLjnLBbRmU+b umXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=Nnqng4e3wmxG5DwoLBgFI4EQ9L7xakS5OJFwO/XXGfc=; b=tD2oI48JTdz5YcQf1QLaxVgkX84Bi0k1T29PPHNRBb0E8q+dBXQZFaXkumzdo4YTUw waWhy+V2Kn6iy5umkpi42gACMn07jCtFus5ld4LWxkzjsDv11zCaa82d2MilLrLCXcH3 XzmIGzL+/d9gjcheX+6fblUFrPVmInezZhlgsjMc1ThYpNgiXlUDwWwR/EDlxOZY67dk AOasfjTqihiqnuA/iJaz89vWXI9m6+bxDRneJONeHGJ2Iur+xW8OPeWeWjtbiQM6JOW+ Zw8jQHK+NTgUoqFphlrF0uMdPMIHa0SC3PzuLS5rv7PctFM1d5mGKi1vFD9SXXAf/euG HoBg== X-Gm-Message-State: APf1xPBQ4QHi1SUWcBt4zn91WciDtkrv8231eenGVyNt2uyOMSM3Kof4 tIbQ4FQJFTV+PlGxHSDNvPmtgB8k X-Google-Smtp-Source: AG47ELuKt05ebbvhdIFQ5A6zcdOqCbyoujynLTM+DProt/PuBH+++qE128kPwYYvSJAKcicSTg8s5g== X-Received: by 10.55.73.140 with SMTP id w134mr23591211qka.215.1519745506903; Tue, 27 Feb 2018 07:31:46 -0800 (PST) Received: from Corsair.corp.extremenetworks.com (cpe-2606-A000-112A-C184-D8EA-C2C6-ED97-896E.dyn6.twc.com. [2606:a000:112a:c184:d8ea:c2c6:ed97:896e]) by smtp.gmail.com with ESMTPSA id t16sm6634334qtj.37.2018.02.27.07.31.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 27 Feb 2018 07:31:46 -0800 (PST) From: Lance Richardson To: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org, lance.richardson.net@gmail.com, mingo@redhat.com Subject: [PATCH v2] fs: select/pselect buffer overrun with x32 abi Date: Tue, 27 Feb 2018 10:31:44 -0500 Message-Id: <20180227153144.4629-1-lance.richardson.net@gmail.com> X-Mailer: git-send-email 2.14.1 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The definition of fd_set in X32 user-space uses a 32-bit base data type for the fd array while the kernel uses a 64-bit base data type. For applications using the glibc implementation of select(2)/pselect(2), the size of fd_set is an integer multiple of both base types, so there is no issue. For applications using fd_set sizes that are different from the glibc default size, an overrun of the user-space fd_set buffer will occur when the user-space buffer size is an odd multiple of 4 bytes (e.g. user-space can pass a 12-byte fd_set to the kernel and the kernel will copy 16 bytes to user-space before returning from select/pselect system calls). OpenSSH is one example of an application using fd_set sizes different from the default. Reproducer (compiled with gcc -mx32): #include #include #include #include #define howmany(x, y) (((x) + (y) - 1) / (y)) int test(int nfds) { fd_set *fds; struct timeval tv = {0, 0}; printf("Calling select with nfds = %d\n", nfds); fds = calloc(howmany(nfds, __NFDBITS), sizeof(__fd_mask)); select(nfds, fds, NULL, NULL, &tv); free(fds); printf("Success\n"); return 0; } int main(int argc, char **argv) { mcheck_pedantic(NULL); test(64); test(65); return 0; } /* Expected output without this patch: * * Calling select with nfds = 64 * Success * Calling select with nfds = 65 * memory clobbered past end of allocated block * Aborted (core dumped) */ Signed-off-by: Lance Richardson --- v2: Fix build breakage when CONFIG_COMPAT is not selected (found by kbuild test robot). fs/select.c | 80 ++++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 25 deletions(-) diff --git a/fs/select.c b/fs/select.c index b6c36254028a..cf4cb5aef6d7 100644 --- a/fs/select.c +++ b/fs/select.c @@ -384,6 +384,33 @@ void zero_fd_set(unsigned long nr, unsigned long *fdset) memset(fdset, 0, FDS_BYTES(nr)); } +#if defined(CONFIG_COMPAT) +/* + * Ooo, nasty. We need here to frob 32-bit unsigned longs to + * 64-bit unsigned longs. + */ +static +int compat_get_fd_set(unsigned long nr, compat_ulong_t __user *ufdset, + unsigned long *fdset) +{ + if (ufdset) { + return compat_get_bitmap(fdset, ufdset, nr); + } else { + zero_fd_set(nr, fdset); + return 0; + } +} + +static +int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset, + unsigned long *fdset) +{ + if (!ufdset) + return 0; + return compat_put_bitmap(ufdset, fdset, nr); +} +#endif + #define FDS_IN(fds, n) (fds->in + n) #define FDS_OUT(fds, n) (fds->out + n) #define FDS_EX(fds, n) (fds->ex + n) @@ -644,10 +671,24 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, fds.res_out = bits + 4*size; fds.res_ex = bits + 5*size; +#if defined(CONFIG_X86_X32) + if (test_thread_flag(TIF_X32)) { + if ((ret = compat_get_fd_set(n, (compat_ulong_t __user *)inp, fds.in)) || + (ret = compat_get_fd_set(n, (compat_ulong_t __user *)outp, fds.out)) || + (ret = compat_get_fd_set(n, (compat_ulong_t __user *)exp, fds.ex))) + goto out; + } else { + if ((ret = get_fd_set(n, inp, fds.in)) || + (ret = get_fd_set(n, outp, fds.out)) || + (ret = get_fd_set(n, exp, fds.ex))) + goto out; + } +#else if ((ret = get_fd_set(n, inp, fds.in)) || (ret = get_fd_set(n, outp, fds.out)) || (ret = get_fd_set(n, exp, fds.ex))) goto out; +#endif zero_fd_set(n, fds.res_in); zero_fd_set(n, fds.res_out); zero_fd_set(n, fds.res_ex); @@ -663,10 +704,24 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, ret = 0; } +#if defined(CONFIG_X86_X32) + if (test_thread_flag(TIF_X32)) { + if ((ret = compat_set_fd_set(n, (compat_ulong_t __user *)inp, fds.res_in)) || + (ret = compat_set_fd_set(n, (compat_ulong_t __user *)outp, fds.res_out)) || + (ret = compat_set_fd_set(n, (compat_ulong_t __user *)exp, fds.res_ex))) + ret = -EFAULT; + } else { + if (set_fd_set(n, inp, fds.res_in) || + set_fd_set(n, outp, fds.res_out) || + set_fd_set(n, exp, fds.res_ex)) + ret = -EFAULT; + } +#else if (set_fd_set(n, inp, fds.res_in) || set_fd_set(n, outp, fds.res_out) || set_fd_set(n, exp, fds.res_ex)) ret = -EFAULT; +#endif out: if (bits != stack_fds) @@ -1149,31 +1204,6 @@ int compat_poll_select_copy_remaining(struct timespec64 *end_time, void __user * return ret; } -/* - * Ooo, nasty. We need here to frob 32-bit unsigned longs to - * 64-bit unsigned longs. - */ -static -int compat_get_fd_set(unsigned long nr, compat_ulong_t __user *ufdset, - unsigned long *fdset) -{ - if (ufdset) { - return compat_get_bitmap(fdset, ufdset, nr); - } else { - zero_fd_set(nr, fdset); - return 0; - } -} - -static -int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset, - unsigned long *fdset) -{ - if (!ufdset) - return 0; - return compat_put_bitmap(ufdset, fdset, nr); -} - /* * This is a virtual copy of sys_select from fs/select.c and probably