Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758698AbcC3Hsj (ORCPT ); Wed, 30 Mar 2016 03:48:39 -0400 Received: from mleia.com ([178.79.152.223]:42588 "EHLO mail.mleia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753129AbcC3Hsh (ORCPT ); Wed, 30 Mar 2016 03:48:37 -0400 Subject: Re: [PATCH v2] USB: whci-hcd: add more checks for dma mapping error To: Alexey Khoroshilov , Greg Kroah-Hartman References: <1459021337-7025-1-git-send-email-khoroshilov@ispras.ru> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, ldv-project@linuxtesting.org From: Vladimir Zapolskiy Message-ID: <56FB84D2.2010806@mleia.com> Date: Wed, 30 Mar 2016 10:48:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <1459021337-7025-1-git-send-email-khoroshilov@ispras.ru> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-49551924 X-CRM114-CacheID: sfid-20160330_085302_142059_0A859E41 X-CRM114-Status: GOOD ( 22.30 ) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2452 Lines: 66 Hi Alexey, On 26.03.2016 21:42, Alexey Khoroshilov wrote: > Fixing checks for dma mapping error in qset_fill_page_list(), > I have missed two similar problems in qset_add_urb_sg() and > in qset_add_urb_sg_linearize(). > > v2: check validity of dma_addr with dma_mapping_error() > in qset_free_std() as suggested by Vladimir Zapolskiy. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov Reviewed-by: Vladimir Zapolskiy > --- > drivers/usb/host/whci/qset.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c > index 1a8e960d073b..c0e6812426b3 100644 > --- a/drivers/usb/host/whci/qset.c > +++ b/drivers/usb/host/whci/qset.c > @@ -314,7 +314,7 @@ void qset_free_std(struct whc *whc, struct whc_std *std) > kfree(std->bounce_buf); > } > if (std->pl_virt) { > - if (std->dma_addr) > + if (!dma_mapping_error(whc->wusbhc.dev, std->dma_addr)) > dma_unmap_single(whc->wusbhc.dev, std->dma_addr, > std->num_pointers * sizeof(struct whc_page_list_entry), > DMA_TO_DEVICE); > @@ -535,9 +535,11 @@ static int qset_add_urb_sg(struct whc *whc, struct whc_qset *qset, struct urb *u > list_for_each_entry(std, &qset->stds, list_node) { > if (std->ntds_remaining == -1) { > pl_len = std->num_pointers * sizeof(struct whc_page_list_entry); > - std->ntds_remaining = ntds--; > std->dma_addr = dma_map_single(whc->wusbhc.dev, std->pl_virt, > pl_len, DMA_TO_DEVICE); > + if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr)) > + return -EFAULT; > + std->ntds_remaining = ntds--; > } > } > return 0; > @@ -618,6 +620,8 @@ static int qset_add_urb_sg_linearize(struct whc *whc, struct whc_qset *qset, > > std->dma_addr = dma_map_single(&whc->umc->dev, std->bounce_buf, std->len, > is_out ? DMA_TO_DEVICE : DMA_FROM_DEVICE); > + if (dma_mapping_error(&whc->umc->dev, std->dma_addr)) > + return -EFAULT; >From whc_probe() looks like &whc->umc->dev is the same as whc->wusbhc.dev, so the change is correct, but I would suggest to unify the pointer to a device. Still the driver has many problems, e.g. double kfree() -- error path in qset_fill_page_list() and qset_free_stds() etc. > if (qset_fill_page_list(whc, std, mem_flags) < 0) > return -ENOMEM; > -- With best wishes, Vladimir