From patchwork Tue Nov 29 19:26:30 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xi Wang X-Patchwork-Id: 128327 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 A959AB6F80 for ; Wed, 30 Nov 2011 06:26:57 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755893Ab1K2T0g (ORCPT ); Tue, 29 Nov 2011 14:26:36 -0500 Received: from mail-qw0-f46.google.com ([209.85.216.46]:48432 "EHLO mail-qw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753633Ab1K2T0e convert rfc822-to-8bit (ORCPT ); Tue, 29 Nov 2011 14:26:34 -0500 Received: by qadc14 with SMTP id c14so2379688qad.19 for ; Tue, 29 Nov 2011 11:26:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; bh=EjNgz6W6AADAjSO6R3xoCf8mVW9ZOse9or6foG7WBJA=; b=X+hozDaCCZci0cZctcAPy1I4Mvu28Bccu4cClsrhHM/ZbLg3JXVOaumVwSYOYfPKO6 Y/44CLKUe0CxU8drtx3Jf2JlyZpkWTGCJVSHXjyo0mTdX0//JPy3ijnCd3oI8rmJG4zP wzpFcIjC1akJjJ2jRHSbRL7q8hRtTHcSi6IXc= Received: by 10.224.183.65 with SMTP id cf1mr6764153qab.55.1322594794006; Tue, 29 Nov 2011 11:26:34 -0800 (PST) Received: from 26-4-169.dynamic.csail.mit.edu (26-4-169.dynamic.csail.mit.edu. [18.26.4.169]) by mx.google.com with ESMTPS id z1sm38682481qao.1.2011.11.29.11.26.32 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 29 Nov 2011 11:26:32 -0800 (PST) Subject: [PATCH v2] sctp: better integer overflow check in sctp_auth_create_key() Mime-Version: 1.0 (Apple Message framework v1084) From: Xi Wang In-Reply-To: Date: Tue, 29 Nov 2011 14:26:30 -0500 Cc: linux-kernel@vger.kernel.org, Sridhar Samudrala , "David S. Miller" , linux-sctp@vger.kernel.org, netdev@vger.kernel.org, security@kernel.org Message-Id: <125BB325-72D4-4FEF-A5CC-118680EC78D2@gmail.com> References: <426D7BA8-ECD0-44D6-A09F-2033F0C825FC@gmail.com> <4ED3AC7D.6090108@hp.com> <147F953A-CC69-41FF-ACD4-64E5E2956411@gmail.com> <4ED4F428.9030807@hp.com> To: Vladislav Yasevich X-Mailer: Apple Mail (2.1084) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The check from commit 30c2235c is incomplete and cannot prevent cases like key_len = 0x80000000 (INT_MAX + 1). In that case, the left-hand side of the check (INT_MAX - key_len), which is unsigned, becomes 0xffffffff (UINT_MAX) and bypasses the check. However this shouldn't be a security issue. The function is called from the following two code paths: 1) setsockopt() 2) sctp_auth_asoc_set_secret() In case (1), sca_keylength is never going to exceed 65535 since it's bounded by a u16 from the user API. As such, the key length will never overflow. In case (2), sca_keylength is computed based on the user key (1 short) and 2 * key_vector (3 shorts) for a total of 7 * USHRT_MAX, which still will not overflow. In other words, this overflow check is not really necessary. Just make it more correct. Signed-off-by: Xi Wang Cc: Vlad Yasevich --- net/sctp/auth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/sctp/auth.c b/net/sctp/auth.c index 865e68f..bf81204 100644 --- a/net/sctp/auth.c +++ b/net/sctp/auth.c @@ -82,7 +82,7 @@ static struct sctp_auth_bytes *sctp_auth_create_key(__u32 key_len, gfp_t gfp) struct sctp_auth_bytes *key; /* Verify that we are not going to overflow INT_MAX */ - if ((INT_MAX - key_len) < sizeof(struct sctp_auth_bytes)) + if (key_len > (INT_MAX - sizeof(struct sctp_auth_bytes))) return NULL; /* Allocate the shared key */