From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sat, 12 Nov 2016 19:17:23 +0100 Subject: [U-Boot] [PATCH] usb: check udev before dereferencing In-Reply-To: <20161112191028.6acc5536@crub> References: <1478941329-9539-1-git-send-email-agust@denx.de> <7b8b75cd-4d6f-3ff0-6fb5-649252e5bc8c@denx.de> <20161112191028.6acc5536@crub> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/12/2016 07:10 PM, Anatolij Gustschin wrote: > On Sat, 12 Nov 2016 10:36:42 +0100 > Marek Vasut marex at denx.de wrote: > ... >>> udev = dev_get_parent_priv(child); >>> + if (!udev) >>> + continue; >> >> I don't quite understand the problem from the patch description, but >> shouldn't all the return values from dev_get_parent_priv() be checked >> this way , not just these two ? > > The problem is that when dereferencing NULL udev we later access > some random address (e.g. when accessing dev->dev->parent in > usb_show_tree_graph()). dev->dev pointer is random DRAM data there, > when dereferencing it, data abort happens when random address > is outside of valid address range. I mean, I understand that udev can be NULL and we don't check it. But is udev == NULL an expected possibility ? And if so, when does such thing happen ? > Probably we should check elsewhere, at least where it might > return NULL. OK >> >> Why does dev_get_parent_priv() return NULL here ? > > it returns NULL because the dev->parent_priv is not allocated for > usb_mass_storage.lun0 device. I do not know the reason why. That's probably what needs to be fixed , no ? Also, we should most likely check all the return values of dev_get_parent_priv() in cmd/usb.c, not just these two. -- Best regards, Marek Vasut