Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752197AbdGFPsj (ORCPT ); Thu, 6 Jul 2017 11:48:39 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:32784 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751070AbdGFPsh (ORCPT ); Thu, 6 Jul 2017 11:48:37 -0400 Date: Thu, 6 Jul 2017 11:48:31 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Colin King cc: Greg Kroah-Hartman , , , , Subject: Re: [PATCH][V2] usb: storage: return on error to avoid a null pointer dereference In-Reply-To: <20170706150633.11240-1-colin.king@canonical.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1416 Lines: 42 On Thu, 6 Jul 2017, Colin King wrote: > From: Colin Ian King > > When us->extra is null the driver is not initialized, however, a > later call to osd200_scsi_to_ata is made that dereferences > us->extra, causing a null pointer dereference. The code > currently detects and reports that the driver is not initialized; > add a return to avoid the subsequent dereference issue in this > check. > > Thanks to Alan Stern for pointing out that srb->result needs setting > to DID_ERROR << 16 > > Detected by CoverityScan, CID#100308 ("Dereference after null check") > > Signed-off-by: Colin Ian King Acked-by: Alan Stern > --- > drivers/usb/storage/isd200.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c > index fba4005dd737..6a7720e66595 100644 > --- a/drivers/usb/storage/isd200.c > +++ b/drivers/usb/storage/isd200.c > @@ -1529,8 +1529,11 @@ static void isd200_ata_command(struct scsi_cmnd *srb, struct us_data *us) > > /* Make sure driver was initialized */ > > - if (us->extra == NULL) > + if (us->extra == NULL) { > usb_stor_dbg(us, "ERROR Driver not initialized\n"); > + srb->result = DID_ERROR << 16; > + return; > + } > > scsi_set_resid(srb, 0); > /* scsi_bufflen might change in protocol translation to ata */ >