[{"id":1767925,"web_url":"http://patchwork.ozlabs.org/comment/1767925/","msgid":"<20170913142855.GA4320@hmswarspite.think-freely.org>","list_archive_url":null,"date":"2017-09-13T14:28:55","subject":"Re: [PATCH net] sctp: potential read out of bounds in\n\tsctp_ulpevent_type_enabled()","submitter":{"id":411,"url":"http://patchwork.ozlabs.org/api/people/411/","name":"Neil Horman","email":"nhorman@tuxdriver.com"},"content":"On Wed, Sep 13, 2017 at 12:20:28PM +0300, Dan Carpenter wrote:\n> This code causes a static checker warning because Smatch doesn't trust\n> anything that comes from skb->data. I've reviewed this code and I do\n> think skb->data can be controlled by the user here.\n> \n> The sctp_event_subscribe struct has 13 __u8 fields and we want to see\n> if ours is non-zero. sn_type can be any value in the 0-USHRT_MAX range.\n> We're subtracting SCTP_SN_TYPE_BASE which is 1 << 15 so we could read\n> either before the start of the struct or after the end.\n> \n> This is a very old bug and it's surprising that it would go undetected\n> for so long but my theory is that it just doesn't have a big impact so\n> it would be hard to notice.\n> \n> Signed-off-by: Dan Carpenter \n> ---\n> I'm not terribly familiar with sctp and this is a static checker fix.\n> Please review it carefully.\n> \n> diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h\n> index 1060494ac230..e6873176bea7 100644\n> --- a/include/net/sctp/ulpevent.h\n> +++ b/include/net/sctp/ulpevent.h\n> @@ -154,7 +154,11 @@ static inline int sctp_ulpevent_type_enabled(__u16 sn_type,\n> \t\t\t\t\t struct sctp_event_subscribe *mask)\n> {\n> \tchar *amask = (char *) mask;\n> -\treturn amask[sn_type - SCTP_SN_TYPE_BASE];\n> +\tint offset = sn_type - SCTP_SN_TYPE_BASE;\n> +\n> +\tif (offset >= sizeof(struct sctp_event_subscribe))\n> +\t\treturn 0;\n> +\treturn amask[offset];\n> }\nAcked-by: Neil Horman \n\n> \n> /* Given an event subscription, is this event enabled? */\n> --\n> To unsubscribe from this list: send the line \"unsubscribe linux-sctp\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xskZ83Xpqz9sNr\n\tfor ;\n\tThu, 14 Sep 2017 00:29:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751590AbdIMO3R (ORCPT );\n\tWed, 13 Sep 2017 10:29:17 -0400","from charlotte.tuxdriver.com ([70.61.120.58]:58845 \"EHLO\n\tsmtp.tuxdriver.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750993AbdIMO3Q (ORCPT\n\t); Wed, 13 Sep 2017 10:29:16 -0400","from [2606:a000:111b:41f4:2201:d2d7:1fb:4ffb] (helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256)\n\t(Exim 4.63) (envelope-from )\n\tid 1ds8eu-0001Dt-5Q; Wed, 13 Sep 2017 10:29:07 -0400"],"Date":"Wed, 13 Sep 2017 10:28:55 -0400","From":"Neil Horman ","To":"Dan Carpenter ","Cc":"Vlad Yasevich ,\n\t\"David S. Miller\" ,\n\tlinux-sctp@vger.kernel.org, netdev@vger.kernel.org,\n\tkernel-janitors@vger.kernel.org","Subject":"Re: [PATCH net] sctp: potential read out of bounds in\n\tsctp_ulpevent_type_enabled()","Message-ID":"<20170913142855.GA4320@hmswarspite.think-freely.org>","References":"<20170913092028.idzvduj7ran4li6b@mwanda>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170913092028.idzvduj7ran4li6b@mwanda>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Spam-Score":"-2.9 (--)","X-Spam-Status":"No","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1768007,"web_url":"http://patchwork.ozlabs.org/comment/1768007/","msgid":"<20170913.092522.934509429497822082.davem@davemloft.net>","list_archive_url":null,"date":"2017-09-13T16:25:22","subject":"Re: [PATCH net] sctp: potential read out of bounds in\n\tsctp_ulpevent_type_enabled()","submitter":{"id":15,"url":"http://patchwork.ozlabs.org/api/people/15/","name":"David Miller","email":"davem@davemloft.net"},"content":"From: Dan Carpenter \nDate: Wed, 13 Sep 2017 12:20:28 +0300\n\n> @@ -154,7 +154,11 @@ static inline int sctp_ulpevent_type_enabled(__u16 sn_type,\n> \t\t\t\t\t struct sctp_event_subscribe *mask)\n> {\n> \tchar *amask = (char *) mask;\n> -\treturn amask[sn_type - SCTP_SN_TYPE_BASE];\n> +\tint offset = sn_type - SCTP_SN_TYPE_BASE;\n\nPlease use reverse-christmas-tree local variable ordering.\n\nThank you.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsn872RjJz9s7M\n\tfor ;\n\tThu, 14 Sep 2017 02:25:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751861AbdIMQZZ (ORCPT );\n\tWed, 13 Sep 2017 12:25:25 -0400","from shards.monkeyblade.net ([184.105.139.130]:51510 \"EHLO\n\tshards.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751457AbdIMQZX (ORCPT\n\t); Wed, 13 Sep 2017 12:25:23 -0400","from localhost (74-93-104-98-Washington.hfc.comcastbusiness.net\n\t[74.93.104.98]) (using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(Client did not present a certificate)\n\t(Authenticated sender: davem-davemloft)\n\tby shards.monkeyblade.net (Postfix) with ESMTPSA id 49A5F12D6DCBC;\n\tWed, 13 Sep 2017 09:25:23 -0700 (PDT)"],"Date":"Wed, 13 Sep 2017 09:25:22 -0700 (PDT)","Message-Id":"<20170913.092522.934509429497822082.davem@davemloft.net>","To":"dan.carpenter@oracle.com","Cc":"vyasevich@gmail.com, nhorman@tuxdriver.com,\n\tlinux-sctp@vger.kernel.org, netdev@vger.kernel.org,\n\tkernel-janitors@vger.kernel.org","Subject":"Re: [PATCH net] sctp: potential read out of bounds in\n\tsctp_ulpevent_type_enabled()","From":"David Miller ","In-Reply-To":"<20170913092028.idzvduj7ran4li6b@mwanda>","References":"<20170913092028.idzvduj7ran4li6b@mwanda>","X-Mailer":"Mew version 6.7 on Emacs 25.2 / Mule 6.0 (HANACHIRUSATO)","Mime-Version":"1.0","Content-Type":"Text/Plain; charset=us-ascii","Content-Transfer-Encoding":"7bit","X-Greylist":"Sender succeeded SMTP AUTH, not delayed by\n\tmilter-greylist-4.5.12 (shards.monkeyblade.net\n\t[149.20.54.216]); Wed, 13 Sep 2017 09:25:23 -0700 (PDT)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]