From patchwork Thu Jul 28 16:37:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarod Wilson X-Patchwork-Id: 9251491 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 76AD3607D8 for ; Thu, 28 Jul 2016 16:37:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6726D20747 for ; Thu, 28 Jul 2016 16:37:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5BC9427D4D; Thu, 28 Jul 2016 16:37:22 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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 B389620747 for ; Thu, 28 Jul 2016 16:37:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932577AbcG1QhV (ORCPT ); Thu, 28 Jul 2016 12:37:21 -0400 Received: from mail-wm0-f47.google.com ([74.125.82.47]:38858 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932535AbcG1QhU (ORCPT ); Thu, 28 Jul 2016 12:37:20 -0400 Received: by mail-wm0-f47.google.com with SMTP id o80so116412519wme.1 for ; Thu, 28 Jul 2016 09:37:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=35buti2bIRbjtsHRYssYItmBeo5yp9mWhfV8g6/G4Wc=; b=VGc/D4KG98oFXfOKHjqBuhghUiOYC9z9HbWg2Oeb4b7VSTXuS4gozCv4SSE5IUXSM0 pUXHSeMFXwVBt3hrUsH0oROdQD/qgR1C37JMEOPUyfXAalJ498RtRSH0Doq2cVhtQnR3 v/y1kzDFqgrIE+kuL75V/etz4FExoNpBwWPBHWBHwDcsjpde6+6gPW+A6uecrFvI8lVK RNuSqzxqrH+b2U2C7eN8PdhNSttMux86+vCQWNZEp6er99cN44w2B3ODHsNY63VPZmvr 9qNomWyeHY6aAKE3IfAkCqDUUR/qDHwcrL5Oz7M7TKQFiMAO1dglX6BK6C5YOi/0V269 e8xQ== X-Gm-Message-State: AEkoouu/pw1GQHk+eXzisf/Hjg0q1eHfO+AP2IrPOKTd7Q+8yAAv9nxwaTMB0EXglltH5WaZ X-Received: by 10.194.89.129 with SMTP id bo1mr5576907wjb.105.1469723838714; Thu, 28 Jul 2016 09:37:18 -0700 (PDT) Received: from redhat.com (nat-pool-bos-t.redhat.com. [66.187.233.206]) by smtp.gmail.com with ESMTPSA id a194sm44200674wmd.24.2016.07.28.09.37.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Jul 2016 09:37:17 -0700 (PDT) Date: Thu, 28 Jul 2016 12:37:14 -0400 From: Jarod Wilson To: Yishai Hadas Cc: linux-rdma@vger.kernel.org, Yishai Hadas Subject: Re: [PATCH libmlx5 2/6] fix coverity buffer overrun warning Message-ID: <20160728163714.GP36313@redhat.com> References: <1469647047-7544-1-git-send-email-jarod@redhat.com> <1469647047-7544-3-git-send-email-jarod@redhat.com> <9ee81879-93c4-97ee-eebf-3300533e4efe@dev.mellanox.co.il> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <9ee81879-93c4-97ee-eebf-3300533e4efe@dev.mellanox.co.il> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Thu, Jul 28, 2016 at 05:46:16PM +0300, Yishai Hadas wrote: > On 7/27/2016 10:17 PM, Jarod Wilson wrote: > >In set_umr_data_seg, there's a union between a 16-byte struct and a > >64-byte array, named data. The code then makes a memset() call on the > >struct that is sizeof(array) - sizeof(struct) long, which results in > >writing 48 bytes to a 16 byte container. Technically, we know this is > >actually fine, because of the union, but to silence the warning, we can > >just do the memset on the array instead. Same address, same result, but no > >warning spew from coverity. > > > >CC: Yishai Hadas > >Signed-off-by: Jarod Wilson > >--- > > src/qp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/src/qp.c b/src/qp.c > >index 51e1176..8bb66be 100644 > >--- a/src/qp.c > >+++ b/src/qp.c > >@@ -426,7 +426,7 @@ static void set_umr_data_seg(struct mlx5_qp *qp, enum ibv_mw_type type, > > data->klm.mkey = htonl(bind_info->mr->lkey); > > data->klm.address = htonll(bind_info->addr); > > > >- memset(&data->klm + 1, 0, sizeof(data->reserved) - > >+ memset(&data->reserved + 1, 0, sizeof(data->reserved) - > > sizeof(data->klm)); > > As you pointed out this is false alarm, code is correct. > > Your suggestion seems wrong as it skipped size of 'reserved' instead > of size of 'klm' (i.e. 16 bytes), isn't it ? &data->klm + 1 and &data->reserved + 1 should point to the same location, no? Must admit, I'm a little hazy on how a union pointer works. Regardless, I think this might be much more straight-forward if we did something like this: From 43e845cc1055e4f86a2e9c78e5a3eae2c1e915c4 Mon Sep 17 00:00:00 2001 From: Jarod Wilson Date: Wed, 27 Jul 2016 11:35:02 -0400 Subject: [PATCH v2 libmlx5 2/6] fix coverity buffer overrun warning In set_umr_data_seg, there's a union between a 16-byte struct and a 64-byte array, named data. The code then makes a memset() call on the struct that is sizeof(array) - sizeof(struct) long, which results in writing 48 bytes to a 16 byte container. Technically, we know this is actually fine, because of the union, but to silence the warning and simplify the pointer math and union gymnastics, simply zero out the entire storage space before assigning klm bits. Signed-off-by: Jarod Wilson --- src/qp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qp.c b/src/qp.c index 51e1176..17f2c81 100644 --- a/src/qp.c +++ b/src/qp.c @@ -422,13 +422,12 @@ static void set_umr_data_seg(struct mlx5_qp *qp, enum ibv_mw_type type, uint8_t reserved[64]; } *data = *seg; + memset(&data, 0, sizeof(*data)); + data->klm.byte_count = htonl(bind_info->length); data->klm.mkey = htonl(bind_info->mr->lkey); data->klm.address = htonll(bind_info->addr); - memset(&data->klm + 1, 0, sizeof(data->reserved) - - sizeof(data->klm)); - *seg += sizeof(*data); *size += (sizeof(*data) / 16); }