Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753524AbdLMP6x (ORCPT ); Wed, 13 Dec 2017 10:58:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34622 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753164AbdLMP6w (ORCPT ); Wed, 13 Dec 2017 10:58:52 -0500 Date: Wed, 13 Dec 2017 08:58:47 -0700 From: Alex Williamson To: Peter Xu Cc: iommu@lists.linux-foundation.org, tursulin@ursulin.net, dwmw2@infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] iommu/vt-d: Fix shift overflow in qi_flush_dev_iotlb Message-ID: <20171213085847.3d3245c6@t450s.home> In-Reply-To: <20171213071355.GD7780@xz-mi> References: <20171212224250.22173.74201.stgit@gimli.home> <20171213071355.GD7780@xz-mi> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 13 Dec 2017 15:58:52 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 892 Lines: 26 On Wed, 13 Dec 2017 15:13:55 +0800 Peter Xu wrote: > On Tue, Dec 12, 2017 at 03:43:08PM -0700, Alex Williamson wrote: > > [...] > > > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c > > index 9a7ffd13c7f0..87888b102057 100644 > > --- a/drivers/iommu/dmar.c > > +++ b/drivers/iommu/dmar.c > > @@ -1345,7 +1345,9 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep, > > struct qi_desc desc; > > > > if (mask) { > > - BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1)); > > + BUG_ON((mask > MAX_AGAW_PFN_WIDTH) || > > + ((mask == MAX_AGAW_PFN_WIDTH) && addr) || > > + (addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1))); > > Could it work if we just use 1ULL instead of 1 here? Thanks, In either case we're talking about shifting off the end of the variable, which I understand to be undefined. Right? Thanks, Alex