From patchwork Fri Nov 29 03:24:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Kerr X-Patchwork-Id: 1202327 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47PKdd5pb6z9sR7 for ; Fri, 29 Nov 2019 14:25:25 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=ozlabs.org Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=ozlabs.org header.i=@ozlabs.org header.b="tSy86Pbc"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 47PKdd4QrTzDr84 for ; Fri, 29 Nov 2019 14:25:25 +1100 (AEDT) X-Original-To: linux-fsi@lists.ozlabs.org Delivered-To: linux-fsi@lists.ozlabs.org Received: from ozlabs.org (bilbo.ozlabs.org [203.11.71.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 47PKdQ5wv3zDr7q for ; Fri, 29 Nov 2019 14:25:14 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=ozlabs.org Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=ozlabs.org header.i=@ozlabs.org header.b="tSy86Pbc"; dkim-atps=neutral Received: by ozlabs.org (Postfix, from userid 1023) id 47PKdQ1fhNz9sR7; Fri, 29 Nov 2019 14:25:13 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ozlabs.org; s=201707; t=1574997914; bh=blTqbfE3FxGoWLMQlFpaeUlzOWBLlqs1Oq3G4TVbtd0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tSy86PbcQQzQzfQOWn2CM+iRgGqTHakwsSKZY1Bw2oTgLLE3ga7ogZs2I5UoD70x+ 1DwJH09EKGSYT2dwClPysTJJkRFbJalUYnY8P59FQet3mG5SnVo9Kcv88tgqhqdSPo OO4wxDaYtiWwm6xMqcTkqqBjeJQeSSm/2tD1/T7XIIJI9mu3T5Ra4y2Gb6lm10SOmV vpcPyI6HK07XwAKxlltc3T1EnAQb1Nxwkt02css7WZRlxze6/qYFnHuPygGK0s5lol n2MxTB+9G7+1qgoLjklw4JW3krc+PzEj6lV+lM8vV6xngzY6CgE2ZfRztVrWMynQLi R9nji/g1PA4aA== From: Jeremy Kerr To: Joel Stanley , Alistar Popple , Eddie James , Benjamin Herrenschmidt , Colin King , linux-fsi@lists.ozlabs.org Subject: [PATCH] [PATCH v2] fsi: fix bogus error returns from cfam_read and cfam_write Date: Fri, 29 Nov 2019 11:24:29 +0800 Message-Id: <20191129032429.817-1-jk@ozlabs.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191122233120.110344-1-colin.king@canonical.com> References: <20191122233120.110344-1-colin.king@canonical.com> MIME-Version: 1.0 X-BeenThere: linux-fsi@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Errors-To: linux-fsi-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "linux-fsi" Based on a static analysis report and original patch from Colin Ian King . Currently, we may drop error values from cfam_read and cfam_write. This change returns the actual error on failure, but a partial read/write will take precedence. Addresses-Coverity: ("Unused value") Fixes: d1dcd6782576 ("fsi: Add cfam char devices") Reported-by: Colin Ian King Signed-off-by: Jeremy Kerr --- Colin: thanks for the report and patch. I think this is a more complete fix, as we want to preseve any partial read/write status if a failure happens mid-way through an operation. Let me know if you (or the coverity analysis) have any feedback. --- drivers/fsi/fsi-core.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 71c6f9fef648..3158a78c2e94 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -699,6 +699,8 @@ static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count, if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) return -EINVAL; + rc = 0; + for (total_len = 0; total_len < count; total_len += read_len) { __be32 data; @@ -707,18 +709,22 @@ static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count, rc = fsi_slave_read(slave, off, &data, read_len); if (rc) - goto fail; + break; rc = copy_to_user(buf + total_len, &data, read_len); if (rc) { rc = -EFAULT; - goto fail; + break; } off += read_len; } - rc = count; - fail: + + /* if we've read any data, we want that to be returned in + * preference to an error state */ + if (total_len) + rc = total_len; + *offset = off; - return count; + return rc; } static ssize_t cfam_write(struct file *filep, const char __user *buf, @@ -736,6 +742,8 @@ static ssize_t cfam_write(struct file *filep, const char __user *buf, if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) return -EINVAL; + rc = 0; + for (total_len = 0; total_len < count; total_len += write_len) { __be32 data; @@ -745,17 +753,21 @@ static ssize_t cfam_write(struct file *filep, const char __user *buf, rc = copy_from_user(&data, buf + total_len, write_len); if (rc) { rc = -EFAULT; - goto fail; + break; } rc = fsi_slave_write(slave, off, &data, write_len); if (rc) - goto fail; + break; off += write_len; } - rc = count; - fail: + + /* if we've written any data, we want to indicate that partial write + * instead of any mid-stream error */ + if (total_len) + rc = total_len; + *offset = off; - return count; + return rc; } static loff_t cfam_llseek(struct file *file, loff_t offset, int whence)