From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from zeeaster.vergenet.net (zeeaster.vergenet.net [206.189.110.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 74BE33C3E for ; Thu, 19 Jan 2023 10:55:03 +0000 (UTC) Received: from momiji.horms.nl (2a02-a46e-7b6b-703-d63d-7eff-fe99-ac9d.fixed6.kpn.net [IPv6:2a02:a46e:7b6b:703:d63d:7eff:fe99:ac9d]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by zeeaster.vergenet.net (Postfix) with ESMTPSA id A067A20095; Thu, 19 Jan 2023 10:47:48 +0000 (UTC) Received: by momiji.horms.nl (Postfix, from userid 7100) id F2BF394047A; Thu, 19 Jan 2023 11:47:47 +0100 (CET) Date: Thu, 19 Jan 2023 11:47:47 +0100 From: Simon Horman To: Dan Carpenter Cc: Yuan Can , gregkh@linuxfoundation.org, drv@mailo.com, damm+renesas@opensource.se, linux-staging@lists.linux.dev Subject: Re: [PATCH v2] staging: emxx_udc: Add checks for dma_alloc_coherent() Message-ID: References: <20230119023430.95919-1-yuancan@huawei.com> Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organisation: Horms Solutions BV X-Virus-Scanned: clamav-milter 0.103.7 at zeeaster X-Virus-Status: Clean On Thu, Jan 19, 2023 at 08:17:12AM +0300, Dan Carpenter wrote: > On Thu, Jan 19, 2023 at 02:34:30AM +0000, Yuan Can wrote: > > As the dma_alloc_coherent may return NULL, the return value needs to be > > checked to avoid NULL poineter dereference. > > > > Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") > > Signed-off-by: Yuan Can > > --- > > changes in v2: > > - do the null pointer check right before the memcpy(). > > > > No. No. This should be: > > diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c > index b4e19174bef2..f9765841c4aa 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.c > +++ b/drivers/staging/emxx_udc/emxx_udc.c > @@ -2587,10 +2587,15 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep, > req->unaligned = false; > > if (req->unaligned) { > - if (!ep->virt_buf) > + if (!ep->virt_buf) { > ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE, > &ep->phys_buf, > GFP_ATOMIC | GFP_DMA); > + if (!ep->virt_buf) { > + spin_unlock_irqrestore(&udc->lock, flags); > + return -ENOMEM; > + } > + } > if (ep->epnum > 0) { > if (ep->direct == USB_DIR_IN) > memcpy(ep->virt_buf, req->req.buf, > Yes, FWIIW, I think so too. Reviewed-by: Simon Horman