From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750829AbdBOFG6 (ORCPT ); Wed, 15 Feb 2017 00:06:58 -0500 Received: from gateway32.websitewelcome.com ([192.185.144.98]:21580 "EHLO gateway32.websitewelcome.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750718AbdBOFG5 (ORCPT ); Wed, 15 Feb 2017 00:06:57 -0500 Date: Tue, 14 Feb 2017 23:06:52 -0600 From: "Gustavo A. R. Silva" To: stern@rowland.harvard.edu, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net, linux-kernel@vger.kernel.org, Peter Senna Tschudi , "Gustavo A. R. Silva" Subject: usb: storage: suspicious code Message-ID: <20170215050652.GA6371@embeddedgus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - gator4166.hostgator.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - embeddedor.com X-BWhitelist: no X-Source-IP: 189.152.159.99 X-Exim-ID: 1cdrnp-000LgE-1K X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: (embeddedgus) [189.152.159.99]:56836 X-Source-Auth: garsilva@embeddedor.com X-Email-Count: 5 X-Source-Cap: Z3V6aWRpbmU7Z3V6aWRpbmU7Z2F0b3I0MTY2Lmhvc3RnYXRvci5jb20= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, I ran into the following piece of code at drivers/usb/storage/jumpshot.c:305 (linux-next), and it seems a little bit suspicious: // read the result. apparently the bulk write can complete // before the jumpshot drive is finished writing. so we loop // here until we get a good return code waitcount = 0; do { result = jumpshot_get_status(us); if (result != USB_STOR_TRANSPORT_GOOD) { // I have not experimented to find the smallest value. // msleep(50); } } while ((result != USB_STOR_TRANSPORT_GOOD) && (waitcount < 10)); if (result != USB_STOR_TRANSPORT_GOOD) usb_stor_dbg(us, "Gah! Waitcount = 10. Bad write!?\n"); Variable 'waitcount' is never updated inside the do-while loop. So, either it isn't needed at all or line 316 should be modified (++waitcount < 10) In case 'waitcount' isn't needed, lines 318 and 319 should be removed. Can someone help me to clarify this so I can write a patch to fix this code? Thank you -- Gustavo A. R. Silva