Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1434485AbdDZIav (ORCPT ); Wed, 26 Apr 2017 04:30:51 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:37935 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1434467AbdDZIag (ORCPT ); Wed, 26 Apr 2017 04:30:36 -0400 Date: Wed, 26 Apr 2017 16:25:26 +0800 From: Jisheng Zhang To: Stefan Wahren CC: Peter Chen , "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH] usb: chipidea: udc: fix NULL pointer dereference if udc_start failed Message-ID: <20170426162526.7c8ef827@xhacker> In-Reply-To: <0ac4bc53-dd26-0b90-c84e-a94d01a6d5e4@i2se.com> References: <20170424123551.2465-1-jszhang@marvell.com> <20170425082948.GB873@b29397-desktop> <20170425171134.6a983841@xhacker> <0ac4bc53-dd26-0b90-c84e-a94d01a6d5e4@i2se.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-04-26_06:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1704260153 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1829 Lines: 52 On Tue, 25 Apr 2017 17:21:59 +0200 Stefan Wahren wrote: > Am 25.04.2017 um 11:20 schrieb Peter Chen: > > > >>>> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > >>>> index f88e9157fad0..60a786c87c06 100644 > >>>> --- a/drivers/usb/chipidea/udc.c > >>>> +++ b/drivers/usb/chipidea/udc.c > >>>> @@ -1984,6 +1984,7 @@ static void udc_id_switch_for_host(struct > >>>> ci_hdrc *ci) int ci_hdrc_gadget_init(struct ci_hdrc *ci) { > >>>> struct ci_role_driver *rdrv; > >>>> + int ret; > >>>> > >>>> if (!hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC)) > >>>> return -ENXIO; > >>>> @@ -1996,7 +1997,10 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci) > >>>> rdrv->stop = udc_id_switch_for_host; > >>>> rdrv->irq = udc_irq; > >>>> rdrv->name = "gadget"; > >>>> - ci->roles[CI_ROLE_GADGET] = rdrv; > >>>> > >>>> - return udc_start(ci); > >>>> + ret = udc_start(ci); > >>>> + if (!ret) > >>>> + ci->roles[CI_ROLE_GADGET] = rdrv; > >>>> + > >>>> + return ret; > >>>> } > >>>> -- > >>> Thanks for fixing it. In fact, we'd better return failure if ret && > >>> ret != -ENXIO at probe, it stands for initialization for host or > >>> gadget has failed. > >>> > >> I got your meaning. I'll cook v2. I don't have preference, since either one can fix the > >> issue. > >> > > Both are needed, you don't need to send this one again. Only a new one, thanks. > > I'm not sure how easy it is to reproduce the issue. It's easy to reproduce it (100%) on arm64 platforms after commit 1dccb598df549 ("arm64: simplify dma_get_ops"). This commit could make all dma related operations failed, then udc_start() would fail with -ENOMEM. On other platforms, it's not easy. > > Shouldn't make a Fixes tag sense at least? maybe 3f124d233e97 ("usb: chipidea: add role init and destroy APIs"