From patchwork Sat Jun 3 05:07:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 9763867 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 2050C602B6 for ; Sat, 3 Jun 2017 05:07:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 12462285E0 for ; Sat, 3 Jun 2017 05:07:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 03663285F9; Sat, 3 Jun 2017 05:07:17 +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=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID 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 510D8285E0 for ; Sat, 3 Jun 2017 05:07:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750876AbdFCFHO (ORCPT ); Sat, 3 Jun 2017 01:07:14 -0400 Received: from mail-it0-f50.google.com ([209.85.214.50]:36048 "EHLO mail-it0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750753AbdFCFHN (ORCPT ); Sat, 3 Jun 2017 01:07:13 -0400 Received: by mail-it0-f50.google.com with SMTP id m47so24024051iti.1 for ; Fri, 02 Jun 2017 22:07:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=+RjXKNS8oOeoKj4NFXKOyojMFs1Esa5Y37VhJ6Eu1Aw=; b=UkBSeKJeyxCgX28xwepQVrknOE3Qi4jb0IWicvtp/gbVkQ/3WZN2hIgKUHbuoq7EJ2 GO4Db21H7bc16xjM1Q2HXU/5jtWpfQp82x43ujFCwNsSA6J8W2X7z5NeJ2FIO4l4R5UO Q81IswZ3BDzuje1A2dxrEtKCBKGdlzmtjaJkZ/sQxpVTt7eEQBCEeIX3jKZGyn2eU1Xi rNRzphwZckRIq2MKWiu3RYOwD4C8VeWOvXfAmLVMgj6o7J8xUH7lJBM1UHK0CSvm3e8e o2nMYE5zUBfkysuam+EtTKQ7hLjSqTzQjQI3chk7kuj2umKInSpxzVlUDOhENMtOIZHJ NQAg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=+RjXKNS8oOeoKj4NFXKOyojMFs1Esa5Y37VhJ6Eu1Aw=; b=dICgqt4/gGRXfl5hYotY9wfSNgoFSKiUH46VaaA/EzzaaKYLuyXBfs8pi4yFyd8wWj u1/CDPPC8rgBWJ+ZIcABZvSLJ+r15tHKbAvrtLkzzMdj1xDFaNWtVavjqilUXZzoLUxD cxzV7jCMuYLRbNRzYPYt1jy3JM1VmuIu3we5E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=+RjXKNS8oOeoKj4NFXKOyojMFs1Esa5Y37VhJ6Eu1Aw=; b=e9Z1vJHYWGmivO5NeVwtPdAlRzis/+Daz9XORU/3kBw3m+UUhxsfqWAzHYNcc88E8v VXzEFnVu0zL7H0RftPV8aJKdrgI+uyssl0IgQS8ZJIeexIKsc8N5XH8bLdzbPe8dBw+d uYb6BsCkPqroT+GokRXL1hKpMn1CouzfDVDTn88u/MtiA6ldj2jn8K2OGSCHAg/X77gQ fPLyuepjPR7m9fm34zKE6zAHr4A1L3pTqac91fKTJTJzDzQijDmbIigZa1H+v+bIEaFC q12nQeDmN1FumA+QfsnFuF2cnOEMMiNZjxrvChaeHOH/e5/Gnz7HS5BDKHI1YC9eoBOc Hfzw== X-Gm-Message-State: AODbwcCihVOdJ8kzkMVH+KAx47nAJrwEh4EYCAXGNRndzRK1YYSyRdmL KDuonu/wFWUeFPt/ku7OxijkDev21ppl X-Received: by 10.107.188.132 with SMTP id m126mr812862iof.148.1496466432865; Fri, 02 Jun 2017 22:07:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.130.148 with HTTP; Fri, 2 Jun 2017 22:07:12 -0700 (PDT) In-Reply-To: <1496439121.13303.1.camel@gmail.com> References: <20170526095404.20439-1-danielmicay@gmail.com> <20170602140743.274b9babba6118bfd12c7a26@linux-foundation.org> <1496439121.13303.1.camel@gmail.com> From: Kees Cook Date: Fri, 2 Jun 2017 22:07:12 -0700 X-Google-Sender-Auth: 8_bH_wo2fHZ-9KSbXjXUMzd4qnw Message-ID: Subject: Re: [PATCH v4] add the option of fortified string.h functions To: Andrew Morton , Moni Shoua , Doug Ledford , Sean Hefty , Hal Rosenstock Cc: Daniel Micay , Linux-MM , "kernel-hardening@lists.openwall.com" , linux-kernel , Mark Rutland , Daniel Axtens , linux-rdma@vger.kernel.org Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jun 2, 2017 at 2:32 PM, Daniel Micay wrote: > On Fri, 2017-06-02 at 14:07 -0700, Andrew Morton wrote: >> On Fri, 26 May 2017 05:54:04 -0400 Daniel Micay > > wrote: >> >> > This adds support for compiling with a rough equivalent to the glibc >> > _FORTIFY_SOURCE=1 feature, providing compile-time and runtime buffer >> > overflow checks for string.h functions when the compiler determines >> > the >> > size of the source or destination buffer at compile-time. Unlike >> > glibc, >> > it covers buffer reads in addition to writes. >> >> Did we find a bug in drivers/infiniband/sw/rxe/rxe_resp.c? >> >> i386 allmodconfig: >> >> In file included from ./include/linux/bitmap.h:8:0, >> from ./include/linux/cpumask.h:11, >> from ./include/linux/mm_types_task.h:13, >> from ./include/linux/mm_types.h:4, >> from ./include/linux/kmemcheck.h:4, >> from ./include/linux/skbuff.h:18, >> from drivers/infiniband/sw/rxe/rxe_resp.c:34: >> In function 'memcpy', >> inlined from 'send_atomic_ack.constprop' at >> drivers/infiniband/sw/rxe/rxe_resp.c:998:2, >> inlined from 'acknowledge' at >> drivers/infiniband/sw/rxe/rxe_resp.c:1026:3, >> inlined from 'rxe_responder' at >> drivers/infiniband/sw/rxe/rxe_resp.c:1286:10: >> ./include/linux/string.h:309:4: error: call to '__read_overflow2' >> declared with attribute error: detected read beyond size of object >> passed as 2nd parameter >> __read_overflow2(); >> >> >> If so, can you please interpret this for the infiniband developers? > > It copies sizeof(skb->cb) bytes with memcpy which is 48 bytes since cb > is a 48 byte char array in `struct sk_buff`. The source buffer is a > `struct rxe_pkt_info`: > > struct rxe_pkt_info { > struct rxe_dev *rxe; /* device that owns packet */ > struct rxe_qp *qp; /* qp that owns packet */ > struct rxe_send_wqe *wqe; /* send wqe */ > u8 *hdr; /* points to bth */ > u32 mask; /* useful info about pkt */ > u32 psn; /* bth psn of packet */ > u16 pkey_index; /* partition of pkt */ > u16 paylen; /* length of bth - icrc */ > u8 port_num; /* port pkt received on */ > u8 opcode; /* bth opcode of packet */ > u8 offset; /* bth offset from pkt->hdr */ > }; > > That looks like 32 bytes (1 byte of padding) on 32-bit and 48 bytes on > 64-bit (1 byte of padding), so on 32-bit there's a read overflow of 16 > bytes from the stack here. This should work (untested): res->atomic.skb = skb; Andrew, there are other fortify fixes too: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/fortify&id=af6b0151896240457ef0fdc18ace533c3d3fbb75 https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/fortify&id=186eaf81b43bf90d6b533732fb11ad31ca27df9d https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/fortify&id=95d589f21b3aef757f0eb3d0224b78648a4b22d2 https://github.com/thestinger/linux-hardened/commit/576e64469b0c4634c007445c5f16bfde610b3600 Do you want me to resend these for you to carry, or reping maintainers? Other fixes have already landed in -next. (And there are two arm64 fixes, too.) -Kees diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index 23039768f541..7b226deb83bb 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -995,7 +995,9 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt, free_rd_atomic_resource(qp, res); rxe_advance_resp_resource(qp); - memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(skb->cb)); + memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(ack_ptr)); + memset(SKB_TO_PKT(skb) + sizeof(ack_ptr), 0, + sizeof(skb->cb) - sizeof(ack_ptr)); res->type = RXE_ATOMIC_MASK;