From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752351AbdATQ6A (ORCPT <rfc822;w@1wt.eu>);
        Fri, 20 Jan 2017 11:58:00 -0500
Received: from mx1.redhat.com ([209.132.183.28]:44502 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751284AbdATQ56 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 20 Jan 2017 11:57:58 -0500
Date: Fri, 20 Jan 2017 14:48:20 -0200
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: colin.king@canonical.com, vyasevich@gmail.com, nhorman@tuxdriver.com,
        linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
Message-ID: <20170120164820.GA669@localhost.localdomain>
References: <20170120134542.21104-1-colin.king@canonical.com>
 <20170120.113117.1592305434267734738.davem@davemloft.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20170120.113117.1592305434267734738.davem@davemloft.net>
User-Agent: Mutt/1.7.1 (2016-10-04)
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 20 Jan 2017 16:48:26 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Jan 20, 2017 at 11:31:17AM -0500, David Miller wrote:
> From: Colin King <colin.king@canonical.com>
> Date: Fri, 20 Jan 2017 13:45:42 +0000
> 
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> > add this in. Also compare timeout with the size of the array
> > sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX.  Also add
> > a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> > so we don't ever get this kind of mismatch between the table
> > and SCTP_EVENT_TIMEOUT_MAX in the future.
> > 
> > Kudos to Marcel Ricardo Leitner for spotting the missing string
> > and suggesting the build time sanity check.
> > 
> > Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
> > 
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> Well, my bad... I reverted V1, that's fine.
> 
> But this patch doesn't even compile.
> 
> In file included from ./include/uapi/linux/stddef.h:1:0,
>                  from ./include/linux/stddef.h:4,
>                  from ./include/uapi/linux/posix_types.h:4,
>                  from ./include/uapi/linux/types.h:13,
>                  from ./include/linux/types.h:5,
>                  from ./include/net/sctp/sctp.h:58,
>                  from net/sctp/debug.c:41:
> net/sctp/debug.c: In function ‘sctp_tname’:
> ./include/linux/compiler.h:518:38: error: call to ‘__compiletime_assert_170’ declared with attribute error: BUILD_BUG_ON failed: SCTP_EVENT_TIMEOUT_MAX + 1 != ARRAY_SIZE(sctp_timer_tbl)
>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)

Seems you applied it on net tree, but commit that introduced the issue
(7b9438de0cd4) is still only on net-next.

I build-tested it here before acking, it worked, on top of
4567d686f5c6d955e57a3afa1741944c1e7f4033.

Colin, please respin the patch.. add the Fixes tag, fix the missing 'o'
in my name on the changelog :-) and tag the patch as net-next tree too.

Thanks,
Marcelo