Received: by 2002:a05:6a10:16a7:0:0:0:0 with SMTP id gp39csp596720pxb; Thu, 12 Nov 2020 11:14:59 -0800 (PST) X-Google-Smtp-Source: ABdhPJwGLEB/Ucrl4uIOiFq2jtoq85bj/yRRkAFzhXSsHn9gDrfbKDR+P0dOE+e0UE2QDXs1sL/F X-Received: by 2002:a05:6402:16d6:: with SMTP id r22mr1503083edx.246.1605208499699; Thu, 12 Nov 2020 11:14:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1605208499; cv=none; d=google.com; s=arc-20160816; b=uuho81w3DzjAWuvMjYw3VMUzqD+6sGgbHhnSADWJ4VlGZGRvZOBtO+tjNhSf39zptD ySsRdE5Kl8tfuyc3C+T7D50Q0O1YK8GQf6qrI6eyak85JyWuFmr4cq4/fbLiphHOoms9 +AzCTvultKEphwgjSRrG+XeaXe3HAidOqWW0YcqHW5w7KFqsoZ6G3igJJsGkGwOwWBib 8QY6JlGtBqXpSXplzlXQlsLlN0JJnYJwivjxnFdXrmSmEOb6Ty0sggB2omQgU+pBn0Ru +qFlDvV5lSZct7bsblh8vCpIEz+gjwqgKnByyYiciA21TRfBv3DV/cw+5HxRCo9R2Wpk D2XA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=pdSRw9mlDHitGPfQBRUv6P8lG2F84sSOXhcflYqXKJo=; b=bsFzuDBje8xMMPFUUS8fw+cYRsMJShffspXhAxKcFcpCanWSwuLUyjdpz3o4NcbsOQ QIBH6ZTZz838sBIdmr1A4WGPUmGcyl/N8aPS6qfUfSup9fJBlGwNhdHa5+T/mcz2Z3tz g3qNh046CAMLB2bRz0kOeXyA2JrR0Otqcmx0yWBAOGOISr/GICyBqJM7Y7X5ql3yj+eu 9Nc0JeOeEmWVbp/7f+o2IsBcOlc8qWyGgcA3DFv2lcw9GTLKbYSNzU8rvA/yjT/HVspv LyvUsSYwtfPAGpH6NDUH7u9J9NDhEVQum8h4wm0dVrAF6mdN40FBzyKl6ByB22JYkXel GhQw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jVBu+ceL; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id i8si2080211edt.264.2020.11.12.11.14.34; Thu, 12 Nov 2020 11:14:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jVBu+ceL; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726610AbgKLTNH (ORCPT + 99 others); Thu, 12 Nov 2020 14:13:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46848 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726295AbgKLTNG (ORCPT ); Thu, 12 Nov 2020 14:13:06 -0500 Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B431DC0613D1; Thu, 12 Nov 2020 11:13:06 -0800 (PST) Received: by mail-wm1-x343.google.com with SMTP id h62so6467294wme.3; Thu, 12 Nov 2020 11:13:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=pdSRw9mlDHitGPfQBRUv6P8lG2F84sSOXhcflYqXKJo=; b=jVBu+ceL7qUj6u8/X7xubAJKNSY5DbZ2HfkQRBgCL4jTPNrpsiFEiEAvT0tRY7qeK6 mf6yNfh9nlT1CFl//UHXh1KCkjpuZfzIyaglUJ9+4xO22Wkk/myB5Fta9JH6OzvGHbOU eWWQ25wqiwH/kYN8X1sbRbZsGS5qAfha++XnYzyED9rqyYe0pTS55PPnZEgUTR05kmXz L2hCpOfrBaMw6Rg/zwikeUz1DrrAWGy7SHVFxf2V2SraMjdWu1C+Cmm/+MzzgvD7tQqP k9OtkUTWl+VM+G0bsyKUvh4KiqY14AuDPrQm3HtYkYBPTd7uFj8RXe++f7/H70PArCCv tCgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=pdSRw9mlDHitGPfQBRUv6P8lG2F84sSOXhcflYqXKJo=; b=HzpGRz4o0tDcnzlSeifbF3c83TBZB711Y3QdAfclzKJ51NolozCkV/KmHbLlesTNlQ UX7Y3jDOCp6N8VdLPuGC05FBVpFHMJFT4Ylk68gvbRh81dQWttxt5xMgfna2kX1/j8ld 0rWagv8kyiEXgceuu8xaYvGCvNtMcoolotduPpD6zkg1loLXNjrdWXzTo0tXQWk5SCQ3 XHsIamzf1dNVhWMl/fTChlhv3jVCAL1SCLm0Vhf4qKtOohxNf1DDeuvUS49Ikt6EaQZm yCfPePt92aS52zedNjFUNcqb8uoI1tD8MVgZ5gH77i16b+RRjVFyvgYtYo2MIC74x6DW vG3w== X-Gm-Message-State: AOAM532PESp6ueFohq/MfuWO8b2J01IqbwRkmVvsGy4MjkPnWsMgf7Pf V7bMiwLDwjUADVI+5ljuD+o= X-Received: by 2002:a1c:b0c4:: with SMTP id z187mr1066051wme.113.1605208385297; Thu, 12 Nov 2020 11:13:05 -0800 (PST) Received: from felia.fritz.box ([2001:16b8:2d19:d300:649c:b988:4446:6d44]) by smtp.gmail.com with ESMTPSA id 30sm427118wrd.88.2020.11.12.11.13.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Nov 2020 11:13:04 -0800 (PST) From: Lukas Bulwahn To: Alan Stern , Greg Kroah-Hartman , linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net Cc: Tom Rix , Nathan Chancellor , Nick Desaulniers , clang-built-linux@googlegroups.com, kernel-janitors@vger.kernel.org, linux-safety@lists.elisa.tech, linux-kernel@vger.kernel.org, Lukas Bulwahn Subject: [PATCH] USB: storage: avoid use of uninitialized values in error path Date: Thu, 12 Nov 2020 20:12:55 +0100 Message-Id: <20201112191255.13372-1-lukas.bulwahn@gmail.com> X-Mailer: git-send-email 2.17.1 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When usb_stor_bulk_transfer_sglist() returns with USB_STOR_XFER_ERROR, it returns without writing to its parameter *act_len. Further, the two callers of usb_stor_bulk_transfer_sglist(): usb_stor_bulk_srb() and usb_stor_bulk_transfer_sg(), use the passed variable partial without checking the return value. Hence, the uninitialized value of partial is then used in the further execution of those two functions. Clang-analyzer detects this potential control and data flow and warns: drivers/usb/storage/transport.c:469:40: warning: The right operand of '-' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult] scsi_set_resid(srb, scsi_bufflen(srb) - partial); ^ drivers/usb/storage/transport.c:495:15: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign] length_left -= partial; ^ When a transfer error occurs, the *act_len value is probably ignored by the higher layers. But it won't hurt to set it to a valid number, just in case. For the two early-return paths in usb_stor_bulk_transfer_sglist(), the amount of data transferred is 0. So if act_len is not NULL, set *act_len to 0 in those paths. That makes clang-analyzer happy. Proposal was discussed in this mail thread: Link: https://lore.kernel.org/linux-usb/alpine.DEB.2.21.2011112146110.13119@felia/ Signed-off-by: Lukas Bulwahn --- applies cleanly on current master and next-20201112 I did some basic compile testing... Alan, Greg, please pick this minor non-urgent clean-up patch. drivers/usb/storage/transport.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c index 238a8088e17f..5eb895b19c55 100644 --- a/drivers/usb/storage/transport.c +++ b/drivers/usb/storage/transport.c @@ -416,7 +416,7 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe, /* don't submit s-g requests during abort processing */ if (test_bit(US_FLIDX_ABORTING, &us->dflags)) - return USB_STOR_XFER_ERROR; + goto usb_stor_xfer_error; /* initialize the scatter-gather request block */ usb_stor_dbg(us, "xfer %u bytes, %d entries\n", length, num_sg); @@ -424,7 +424,7 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe, sg, num_sg, length, GFP_NOIO); if (result) { usb_stor_dbg(us, "usb_sg_init returned %d\n", result); - return USB_STOR_XFER_ERROR; + goto usb_stor_xfer_error; } /* @@ -452,6 +452,11 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe, *act_len = us->current_sg.bytes; return interpret_urb_result(us, pipe, length, result, us->current_sg.bytes); + +usb_stor_xfer_error: + if (act_len) + *act_len = 0; + return USB_STOR_XFER_ERROR; } /* -- 2.17.1