From patchwork Fri Nov 25 08:30:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiri Slaby X-Patchwork-Id: 699100 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3tQ8fG049qz9t37 for ; Fri, 25 Nov 2016 19:40:22 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753966AbcKYIkK (ORCPT ); Fri, 25 Nov 2016 03:40:10 -0500 Received: from mx2.suse.de ([195.135.220.15]:45240 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752778AbcKYIcZ (ORCPT ); Fri, 25 Nov 2016 03:32:25 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de X-Amavis-Alert: BAD HEADER SECTION, Duplicate header field: "References" Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id AD2F0AE11; Fri, 25 Nov 2016 08:31:43 +0000 (UTC) From: Jiri Slaby To: stable@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Jiri Slaby , Vlad Yasevich , Neil Horman , "David S. Miller" , linux-sctp@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 3.12 093/127] net: sctp, forbid negative length Date: Fri, 25 Nov 2016 09:30:06 +0100 Message-Id: <701a77ec87e209dba3514fb80cfb3e147b9aa56c.1480062521.git.jslaby@suse.cz> X-Mailer: git-send-email 2.10.2 In-Reply-To: References: In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 3.12-stable review patch. If anyone has any objections, please let me know. =============== [ Upstream commit a4b8e71b05c27bae6bad3bdecddbc6b68a3ad8cf ] Most of getsockopt handlers in net/sctp/socket.c check len against sizeof some structure like: if (len < sizeof(int)) return -EINVAL; On the first look, the check seems to be correct. But since len is int and sizeof returns size_t, int gets promoted to unsigned size_t too. So the test returns false for negative lengths. Yes, (-1 < sizeof(long)) is false. Fix this in sctp by explicitly checking len < 0 before any getsockopt handler is called. Note that sctp_getsockopt_events already handled the negative case. Since we added the < 0 check elsewhere, this one can be removed. If not checked, this is the result: UBSAN: Undefined behaviour in ../mm/page_alloc.c:2722:19 shift exponent 52 is too large for 32-bit type 'int' CPU: 1 PID: 24535 Comm: syz-executor Not tainted 4.8.1-0-syzkaller #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014 0000000000000000 ffff88006d99f2a8 ffffffffb2f7bdea 0000000041b58ab3 ffffffffb4363c14 ffffffffb2f7bcde ffff88006d99f2d0 ffff88006d99f270 0000000000000000 0000000000000000 0000000000000034 ffffffffb5096422 Call Trace: [] ? __ubsan_handle_shift_out_of_bounds+0x29c/0x300 ... [] ? kmalloc_order+0x24/0x90 [] ? kmalloc_order_trace+0x24/0x220 [] ? __kmalloc+0x330/0x540 [] ? sctp_getsockopt_local_addrs+0x174/0xca0 [sctp] [] ? sctp_getsockopt+0x10d/0x1b0 [sctp] [] ? sock_common_getsockopt+0xb9/0x150 [] ? SyS_getsockopt+0x1a5/0x270 Signed-off-by: Jiri Slaby Cc: Vlad Yasevich Cc: Neil Horman Cc: "David S. Miller" Cc: linux-sctp@vger.kernel.org Cc: netdev@vger.kernel.org Acked-by: Neil Horman Signed-off-by: David S. Miller Signed-off-by: Jiri Slaby --- net/sctp/socket.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index ead3a8adca08..98cd6606f4a4 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4247,7 +4247,7 @@ static int sctp_getsockopt_disable_fragments(struct sock *sk, int len, static int sctp_getsockopt_events(struct sock *sk, int len, char __user *optval, int __user *optlen) { - if (len <= 0) + if (len == 0) return -EINVAL; if (len > sizeof(struct sctp_event_subscribe)) len = sizeof(struct sctp_event_subscribe); @@ -5758,6 +5758,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname, if (get_user(len, optlen)) return -EFAULT; + if (len < 0) + return -EINVAL; + sctp_lock_sock(sk); switch (optname) {