From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH v3 1/1] iommu/arm-smmu: Do not access non-existing S2CR registers Date: Fri, 22 Aug 2014 16:22:15 +0100 Message-ID: <20140822152215.GC17255@arm.com> References: <1408403858-16766-1-git-send-email-ohaugan@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1408403858-16766-1-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Olav Haugan Cc: "linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-arm-msm@vger.kernel.org Hi Olav, Some really minor comments, but this looks good. On Tue, Aug 19, 2014 at 12:17:38AM +0100, Olav Haugan wrote: > The number of S2CR registers is not properly set when stream > matching is not supported. Fix this and add check that we do not try to > access outside of the number of S2CR regisrers. > > Signed-off-by: Olav Haugan > --- > drivers/iommu/arm-smmu.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 9fd8754d..a050f7a 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -548,9 +548,16 @@ static int register_smmu_master(struct arm_smmu_device *smmu, > master->of_node = masterspec->np; > master->cfg.num_streamids = masterspec->args_count; > > - for (i = 0; i < master->cfg.num_streamids; ++i) > + for (i = 0; i < master->cfg.num_streamids; ++i) { > + if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) && > + (masterspec->args[i] >= smmu->num_mapping_groups)) { Can we stick masterspec->args[i] into a loop-local streamid variable, please? > + dev_err(dev, > + "stream ID for master device %s greater than maximum allowed (%d)\n", > + masterspec->np->name, smmu->num_mapping_groups); > + return -ENOSPC; -ERANGE might be better. Will