Received: by 2002:a05:7412:3b8b:b0:fc:a2b0:25d7 with SMTP id nd11csp1708352rdb; Sat, 10 Feb 2024 13:54:19 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCU9MCtl+8sns1M0ZHPaRw/xPqxaTWpFRkqSmAUeniOmmET/PyVZsppFwZs6nLzP23m2QbJD1F+xs3m1QTFgnYsL8h/x5AIPSs8Tf12QKg== X-Google-Smtp-Source: AGHT+IEUzsjXnEQkDe1YwU6nyIG9G2e+4sJYkTLsowKAoiR/qpjWK8sVmy2QC0DfaFbpQT12traP X-Received: by 2002:a0c:cd0b:0:b0:68c:aac7:35ae with SMTP id b11-20020a0ccd0b000000b0068caac735aemr3142255qvm.58.1707602059100; Sat, 10 Feb 2024 13:54:19 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707602059; cv=pass; d=google.com; s=arc-20160816; b=EuKjZsS/ApMmor64BTDTKoWRK+J03eAjKz+DaLIcPwlOUtz0l1lfo6vhf4N9hxbG71 x7adooBNzEG00N/VLFnEn8isa79DZqP/jQdZ+2wHLZxPJOyFA26nZLJsDw6Z/t96sQ60 R1VSjQteQ5p0Q3Tgaz+g/WEtpPZcaWBE8v3O8P+MNEDlQeq8xy4imURaTm8j0XPoPNX1 Gog8rnKlFyk37uAB1lZouMbCymqNj2NhE1QHjK8/a98OnAQT2E2RK7/K7RJgdlCkP3ud HZXKXUZM4F4nrcICCkfNREEWCsUWwLx/fi9mym54+mnFkOkKG4b63Veib6MIiFWNpg0/ OZTw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=s+VuQ9yysl3g1DNmr5ri0m5tR/HKq6LHQGQTGRbQekY=; fh=9tHGAei62hoJ5r9ZCdOdtHmRWOxM2BP+/kvBpi6p9BE=; b=Y7Arcy1YMd/PrpqpR4ipGlka0WFxPCwdBjuyyMmzGByEc/IVW2zadChwfHmnBXIQqN gxDM7Ye0pMAztqcBx7Gb3IMUr7kJlUvDhjlme5vYfwFhJ1+0uhaiGMXAoqtGia6eem0m 6yNHkP3LGJJ5M1gfs8OzXjljsrSG1+gav5bHPryl3TYlidSKSlf69u9BIpKLHFg65I7j KQ9lhuintCxlZBA4CxJqLd+M/pbSXglAfIa85bQXnO1wGXgKVuisDi/P6SE1TDMHtaaA oEQqCeAC5dWSJ/GVEqFs5oO24AWmbD7HuDaSWaMDpjT2YYXZMhbW5M/nq1A4ZmFQO+E6 piTw==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=efZhc3TJ; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-60552-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-60552-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com X-Forwarded-Encrypted: i=2; AJvYcCX5gQ1+ny8qMg1uysHyoWWQbRPKr+WdJYMiPPmBKZ8w+kxlDYK4S+zklCMU+pnbsrJWs3aWMxIoDse6iKXz0zwAH3jbJkG+ATfMFyAo2g== Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id jz12-20020a0562140e6c00b0068ca0f1faafsi5180660qvb.1.2024.02.10.13.54.18 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 10 Feb 2024 13:54:19 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-60552-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=efZhc3TJ; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-60552-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-60552-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 979361C23483 for ; Sat, 10 Feb 2024 21:54:18 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 790075FEF3; Sat, 10 Feb 2024 21:53:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="efZhc3TJ" Received: from mail-lj1-f179.google.com (mail-lj1-f179.google.com [209.85.208.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C0B716027A; Sat, 10 Feb 2024 21:53:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707601983; cv=none; b=Qqs+RAemtkUC8SmvuA5z/kptIA6XUKSY6P3iX2NokI8wvv9luOcQEGUI3gK25MGlqgzJbVRtXbUWdwUYT5EmIoVjBXO2a1Yg2nusxqh2iGF0n4bnXdNrV9dFpPaTp33MxTLKePsT5pwmIjvzjWzh5n+ou9v+UEPGGvqt7gc8cvE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707601983; c=relaxed/simple; bh=9o7zsXvraSjsCEIhLU7II+BaKhLy7FlP98jc26bW638=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=VGpJ1H/7ZIYO6cjOiS6/viJwnkfNmDqC3sFfHt3+M/Gzmx7gjgpZZZXpY9CFWLRTx0MYmK/yWeY+qG+1v9i6csvwoH/GKlK4IW29m8KMao0ft5pKRON3++aLGyhbgUmp5uEQb80nbVSpzphlfrHd2S6ncdtxP3z4kpnmN9J9goE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=efZhc3TJ; arc=none smtp.client-ip=209.85.208.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-f179.google.com with SMTP id 38308e7fff4ca-2d0a4e1789cso22774381fa.3; Sat, 10 Feb 2024 13:53:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707601980; x=1708206780; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=s+VuQ9yysl3g1DNmr5ri0m5tR/HKq6LHQGQTGRbQekY=; b=efZhc3TJgQTF4sJR8WSMTw1p3qS1CXYCkKDnXoR0sr27FAMSJ1QWY8F4cSOkMhsInQ 28n543rlBSSf1ok+bGgLqkrCusgMJ3Kzq7uvIFeuAbYQd/y9pQe4OZLrhKAsBc8WOWxT 49NOoRmLHJ482yr44uCtXnoH/tOxFt8u01ZkPewveiwBC6+ufyAwsdEPtdvy+DXSS0zO bgM0K/CJW6KPG4AFUIYxNsH5b+tTqouC/nn0p2vWhKQBI8g2WVI68ydpdQ+hlXci9WwV R13QNtHZkyOOJmEOKEPXk/euUMuCsciFOKU7CFQz+qvWrQFFCqzh55Fc8Dhha9C1xdvF Xagw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707601980; x=1708206780; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=s+VuQ9yysl3g1DNmr5ri0m5tR/HKq6LHQGQTGRbQekY=; b=qGyyFkiA8/DVUJPig4HkpNUVH4mWlTtPFZevFDP2cPjHmuicLrv5Tu25z3rhtQS2BN MExpmjuA4fF3LK1paMagLtswliu9HtvBvhQYQ7OL2Cp3e6axFAH6PTTh5a9HU1smghWt 7gbDErZrg8XMHJIqb1fx8QHeIC9riyR558hsHyiVxD4N0egLvIoESsBuSj+JcDjFcJkG 9IXAK8pouo+UKUE1ymZ49STel7UtRK0saUcHa762muGWc/z6gikSoCpWcGUmmmkoUGU+ /H4PscjLhLVSN8Bspk1ToWpXN4ZrVVgj/TFpkOFWXD0uAh+EESJXKxrnQC8PiTVVXX1N tHXg== X-Forwarded-Encrypted: i=1; AJvYcCXOUScT95L3ea628hR3+sAG16U//oKuvofbveQC12LDw8dttYHsgoMv7kO8/nSZ35CeU1T2yRIZGikfXKQEGTofJv49RY66OvrISlShVq1YaYWwgSrbOLzSGv8pcgPEu2crJQ/96vyyMOc= X-Gm-Message-State: AOJu0YwmY0wJWkNGh0wvPVNC/rfIulcaJBs5gGhWs+E9Q39g2ZpvZyrp TUanHMJR16C1HRwu3kEDT6BZzRxWGVv8B9TlFXDv6XB9fNXvnBWe X-Received: by 2002:a19:2d47:0:b0:511:47a4:ef0e with SMTP id t7-20020a192d47000000b0051147a4ef0emr1677909lft.7.1707601979600; Sat, 10 Feb 2024 13:52:59 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCW2cPEPDaaSC/hrzZi+46NnAVWNP88hnIjxa10U8y44az98mEw8yO7NnW0zgknZAIvkOEZePm7dtPrLpXjOSIF/iDY36PUYTcXVdtUpYzpz4d41vs6jD39ZlxLMMFa0o69BbtyBnRVGKT7hv7T3ZJRitxjBEzzdgrnu1niUHF6Tgf3/z/gtqV9H/fAV/vXMez9nGi9/XFAI4Suy+XI64cbyPQU6uzLT0vVwDSmBbDaExoO8owVPhsVD5v8vaaXpHIq/f0jsxX2llD4wCoUVR/wJf6FYsIwYl70loRr4YESmRR9nIzIDg2jPILkxOp+p0chcbry1MnnHHuRx0v0OWT5CGyl/TVfyUh09wKbZa29nS1nXuXZfgw== Received: from m2.. (89-139-223-180.bb.netvision.net.il. [89.139.223.180]) by smtp.googlemail.com with ESMTPSA id a7-20020a19ca07000000b0051189b53f93sm24005lfg.302.2024.02.10.13.52.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 10 Feb 2024 13:52:59 -0800 (PST) From: Michael Zaidman To: chrysh@christina-quast.de, daniel.beer@igorinstitute.com, jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-serial@vger.kernel.org, ilpo.jarvinen@linux.intel.com, johan@kernel.org, gregkh@linuxfoundation.org, equinox@diac24.net, michael.zaidman@gmail.com Subject: [PATCH v1 05/19] hid-ft260: fix unprotected write_buf concurrent access Date: Sat, 10 Feb 2024 23:51:33 +0200 Message-Id: <20240210215147.77629-6-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com> References: <20240210215147.77629-1-michael.zaidman@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit The UART code uses the write_buf unsafely, compromising the data integrity of both I2C and UART channels. The I2C channel uses the write_buf to send the HID reports. It uses mutex to make it atomically. For UART to use this buffer, it should grab the same mutex first. But then it will degrade the performance of both channels. The better approach is to have a separate Tx buffer for UART. I fixed it and briefly tested the data integrity simultaneously writing via I2C and UART channels. Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 77638cae595e..3d6beac0b8b6 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -38,9 +38,12 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); pr_info("%s: " format, __func__, ##arg); \ } while (0) -#define FT260_REPORT_MAX_LENGTH (64) -#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) -#define FT260_UART_DATA_REPORT_ID(len) (FT260_UART_REPORT_MIN + (len - 1) / 4) +#define FT260_REPORT_MAX_LEN (64) +#define FT260_DATA_REPORT_ID(min, len) (min + (len - 1) / 4) +#define FT260_I2C_DATA_REPORT_ID(len) \ + FT260_DATA_REPORT_ID(FT260_I2C_REPORT_MIN, len) +#define FT260_UART_DATA_REPORT_ID(len) \ + FT260_DATA_REPORT_ID(FT260_UART_REPORT_MIN, len) #define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */ @@ -56,7 +59,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); * read payload length to be 180 bytes. */ #define FT260_RD_DATA_MAX (180) -#define FT260_WR_DATA_MAX (60) +#define FT260_WR_I2C_DATA_MAX (60) +#define FT260_WR_UART_DATA_MAX (62) /* * Device interface configuration. @@ -229,7 +233,7 @@ struct ft260_i2c_write_request_report { u8 address; /* 7-bit I2C address */ u8 flag; /* I2C transaction condition */ u8 length; /* data payload length */ - u8 data[FT260_WR_DATA_MAX]; /* data payload */ + u8 data[FT260_WR_I2C_DATA_MAX]; /* data payload */ } __packed; struct ft260_i2c_read_request_report { @@ -249,7 +253,7 @@ struct ft260_input_report { struct ft260_uart_write_request_report { u8 report; /* FT260_UART_REPORT */ u8 length; /* data payload length */ - u8 data[] __counted_by(length); /* variable data payload */ + u8 data[FT260_WR_UART_DATA_MAX]; /* data payload */ } __packed; struct ft260_configure_uart_request { @@ -318,10 +322,10 @@ struct ft260_device { struct work_struct wakeup_work; bool reschedule_work; - struct completion wait; struct mutex lock; - u8 write_buf[FT260_REPORT_MAX_LENGTH]; + u8 i2c_wr_buf[FT260_REPORT_MAX_LEN]; + u8 uart_wr_buf[FT260_REPORT_MAX_LEN]; unsigned long need_wakeup_at; u8 *read_buf; u16 read_idx; @@ -503,7 +507,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, int ret, wr_len, idx = 0; struct hid_device *hdev = dev->hdev; struct ft260_i2c_write_request_report *rep = - (struct ft260_i2c_write_request_report *)dev->write_buf; + (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf; if (len < 1) return -EINVAL; @@ -511,12 +515,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, rep->flag = FT260_FLAG_START; do { - if (len <= FT260_WR_DATA_MAX) { + if (len <= FT260_WR_I2C_DATA_MAX) { wr_len = len; if (flag == FT260_FLAG_START_STOP) rep->flag |= FT260_FLAG_STOP; } else { - wr_len = FT260_WR_DATA_MAX; + wr_len = FT260_WR_I2C_DATA_MAX; } rep->report = FT260_I2C_DATA_REPORT_ID(wr_len); @@ -552,7 +556,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd, int len = 4; struct ft260_i2c_write_request_report *rep = - (struct ft260_i2c_write_request_report *)dev->write_buf; + (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf; if (data_len >= sizeof(rep->data)) return -EINVAL; @@ -1167,10 +1171,10 @@ static int ft260_uart_transmit_chars(struct ft260_device *port) goto tty_out; } - rep = (struct ft260_uart_write_request_report *)port->write_buf; + rep = (struct ft260_uart_write_request_report *)port->uart_wr_buf; do { - len = min(data_len, FT260_WR_DATA_MAX); + len = min(data_len, FT260_WR_UART_DATA_MAX); rep->report = FT260_UART_DATA_REPORT_ID(len); rep->length = len; -- 2.40.1