From patchwork Thu Nov 30 14:28:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ding Hui X-Patchwork-Id: 171941 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp433446vqy; Thu, 30 Nov 2023 06:38:25 -0800 (PST) X-Google-Smtp-Source: AGHT+IE1YX+RUeY9i9pqpRqFTBSkz1rF4ORAlk1D6usdq9jBRHC9FLhj/xsPNLmHQHGIOTR9W5zx X-Received: by 2002:a05:6808:14d5:b0:3b2:e3e5:6b58 with SMTP id f21-20020a05680814d500b003b2e3e56b58mr24364821oiw.12.1701355104962; Thu, 30 Nov 2023 06:38:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701355104; cv=none; d=google.com; s=arc-20160816; b=eLf9tJl1kW+3HiH0k0GwkNDG6+g1iTAzUQK0tOp8CX4l147toy1peBq/BDCpYk77GZ xwKWr/wSD2Yq+/xnct7kZjChPF3GHmqQeFIjhmo1kjQ4Wnh/+Et51mYKwBk1JfeOdaD6 G6Jk5VJTu3Vr1bPDGPCksNxQCq1gNfcuG0TH+Z9EGpLDSxqHTtbSpdY1KQHvIs765fpK H41E/IjTTPmU4Udk0HIPzyOAmL96AGkNpfunZs/x0PbR4Fee8blSp6U3gvrajrc6700R UEB5HV7JS0NJc6v89Q7DdtOlJtkEiuqUrCzmWJYZ/XG4u6u/Bhghcde35VHAFfi2JHiy mNjA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from; bh=pOJjJBWCotNesXaGb4iYnFVTyNFAD+7k2oUwMJMotoE=; fh=LrU4YziiOyTsV/Dd6axnIynt7egzzEBi0uinVswYT9A=; b=ggtiV0CitE9U9B6TuRg4QV/GO2WsIyx276HyS8ieNu2Qt5sfMPaTSfIKaX2WNhabUT 1TnfvLz0Q3mexxN+YAYASGrbwEDIVesC6G3S/OSc/H+lUAtWvupMy3kImjo+uNY6QT6r WKwr/460nGd5L4Wl7/i8Fk93cpnkjR9tGmbE+/SsVJPoXE3SjolwH6yenS1rpQVDkgD6 Ir3ps6dXKutltnw+IH/sQkd0j83rSQYhKixeggWfgkmBGgzNXpXQN4YjzJoZQOJYlSKp t+XCvC1Yu9a7etM5aE4jUV46aGRqBXrAhSZPyLPJApD1XIp9F5JWP1M9Zz3XU1EVndUF GNKQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=sangfor.com.cn Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id 127-20020a630985000000b005bd3ca6c398si1387220pgj.736.2023.11.30.06.38.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Nov 2023 06:38:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=sangfor.com.cn Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 3C356826FA9E; Thu, 30 Nov 2023 06:38:16 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346043AbjK3OiD (ORCPT + 99 others); Thu, 30 Nov 2023 09:38:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43306 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346050AbjK3OiB (ORCPT ); Thu, 30 Nov 2023 09:38:01 -0500 Received: from mail-m118208.qiye.163.com (mail-m118208.qiye.163.com [115.236.118.208]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 94C811B4; Thu, 30 Nov 2023 06:38:06 -0800 (PST) Received: from localhost.localdomain (unknown [IPV6:240e:3b7:3271:7f20:45e9:2b16:3419:6e5b]) by mail-m12773.qiye.163.com (Hmail) with ESMTPA id A0A592C04E9; Thu, 30 Nov 2023 22:28:57 +0800 (CST) From: Ding Hui To: jejb@linux.ibm.com, martin.petersen@oracle.com Cc: zhuwei@sangfor.com.cn, thenzl@redhat.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, Ding Hui Subject: [PATCH 2/2] scsi: ses: increase default init_alloc_size Date: Thu, 30 Nov 2023 22:28:35 +0800 Message-Id: <20231130142835.18041-3-dinghui@sangfor.com.cn> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20231130142835.18041-1-dinghui@sangfor.com.cn> References: <20231130142835.18041-1-dinghui@sangfor.com.cn> X-HM-Spam-Status: e1kfGhgUHx5ZQUpXWQgPGg8OCBgUHx5ZQUlOS1dZFg8aDwILHllBWSg2Ly tZV1koWUFITzdXWS1ZQUlXWQ8JGhUIEh9ZQVkZGksfVkNDTEkaS0hCGEMaQlUTARMWGhIXJBQOD1 lXWRgSC1lBWUlPSx5BSBlMQUhJTEpBTB1JS0FPTh5CQUkZSk1BSE9KQkFNHk4ZWVdZFhoPEhUdFF lBWU9LSFVKTU9JTklVSktLVUpCWQY+ X-HM-Tid: 0a8c20a20383b249kuuua0a592c04e9 X-HM-MType: 1 X-HM-Sender-Digest: e1kMHhlZQR0aFwgeV1kSHx4VD1lBWUc6Ni46LRw4LTw#SxcBIzgRLzgY Fh4KCQlVSlVKTEtKSE5PTkhDTUJNVTMWGhIXVR8SFRwTDhI7CBoVHB0UCVUYFBZVGBVFWVdZEgtZ QVlJT0seQUgZTEFISUxKQUwdSUtBT04eQkFJGUpNQUhPSkJBTR5OGVlXWQgBWUFMSE9KNwY+ X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 30 Nov 2023 06:38:16 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784000130410279365 X-GMAIL-MSGID: 1784000130410279365 In 2020, I sent a patch [1] to address crash due to zero component count, at that time, I did not dig why the ses driver got 0 but sg_ses tool got the right component count. Now we have the answer. In ses, when we prepare to receive a full diagnostic page{1,2,7,10}, we first detect the page length by a RECEIVE_DIAGNOSTIC request with INIT_ALLOC_SIZE buffer len, we expect to get the correct page length regardless of detecting request buffer len. But for some storage device (e.g. vendor:DELL product:MD32xxi rev:0784), its behavior is different. It replies the page length indicating that it actually filled the buffer size in the response, rather that the original page length it should be. In this situation, the device reply hdr_buf[3]=28 since our detecting request buffer is 32, we will allocate small buffer and save partial diag page content, when we parse the page content, multiple types OOB reading could be triggered. This is also the root cause of the OOB handled by Zhu Wei in the previous patch. The sg_ses directly invoke SG_IO ioctl with dxfer_len=65535, so it always can report the right elements and descriptors. To work well with this kind of devices, I increase the default init_alloc_size to a relative large enough length for most devices, and convert it to a module param in case some one need even larger who warned by the log "Suspicious pageX len Y, try larger init_alloc_size". [1]: https://patchwork.kernel.org/patch/11938277 Cc: Zhu Wei Signed-off-by: Ding Hui --- drivers/scsi/ses.c | 51 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 2a404e51b6db..40069a4b51a8 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -35,6 +35,29 @@ struct ses_component { u64 addr; }; +#define INIT_ALLOC_SIZE_DEF 4096 +#define INIT_ALLOC_SIZE_MIN 32 +#define INIT_ALLOC_SIZE_MAX 65535 + +static uint init_alloc_size = INIT_ALLOC_SIZE_DEF; + +static int param_set_init_alloc_size(const char *val, const struct kernel_param *kp) +{ + uint size; + int ret = kstrtouint(val, 0, &size); + + if (ret) + return ret; + + init_alloc_size = clamp_t(uint, size, INIT_ALLOC_SIZE_MIN, INIT_ALLOC_SIZE_MAX); + return 0; +} + +module_param_call(init_alloc_size, param_set_init_alloc_size, param_get_uint, + &init_alloc_size, 0644); +MODULE_PARM_DESC(init_alloc_size, "Buffer size for detecting diagnostic pages length. " + "[Default=" __stringify(INIT_ALLOC_SIZE_DEF) "]"); + static bool ses_page2_supported(struct enclosure_device *edev) { struct ses_device *ses_dev = edev->scratch; @@ -525,8 +548,6 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev, return 0; } -#define INIT_ALLOC_SIZE 32 - static void ses_enclosure_data_process(struct enclosure_device *edev, struct scsi_device *sdev, int create) @@ -536,7 +557,8 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, int i, j, page7_len, len, components; struct ses_device *ses_dev = edev->scratch; int types = ses_dev->page1_num_types; - unsigned char *hdr_buf = kzalloc(INIT_ALLOC_SIZE, GFP_KERNEL); + int hdr_buf_len = init_alloc_size; + unsigned char *hdr_buf = kzalloc(hdr_buf_len, GFP_KERNEL); if (!hdr_buf) goto simple_populate; @@ -545,11 +567,14 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, if (ses_dev->page10) ses_recv_diag(sdev, 10, ses_dev->page10, ses_dev->page10_len); /* Page 7 for the descriptors is optional */ - result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE); + result = ses_recv_diag(sdev, 7, hdr_buf, hdr_buf_len); if (result) goto simple_populate; page7_len = len = (hdr_buf[2] << 8) + hdr_buf[3] + 4; + if (page7_len == hdr_buf_len) + sdev_printk(KERN_WARNING, sdev, + "Suspicious page7 len %d, try larger init_alloc_size\n", page7_len); /* add 1 for trailing '\0' we'll use */ buf = kzalloc(len + 1, GFP_KERNEL); if (!buf) @@ -678,6 +703,7 @@ static int ses_intf_add(struct device *cdev) int num_enclosures; struct enclosure_device *edev; struct ses_component *scomp = NULL; + int hdr_buf_len = init_alloc_size; if (!scsi_device_enclosure(sdev)) { /* not an enclosure, but might be in one */ @@ -695,16 +721,19 @@ static int ses_intf_add(struct device *cdev) sdev_printk(KERN_NOTICE, sdev, "Embedded Enclosure Device\n"); ses_dev = kzalloc(sizeof(*ses_dev), GFP_KERNEL); - hdr_buf = kzalloc(INIT_ALLOC_SIZE, GFP_KERNEL); + hdr_buf = kzalloc(hdr_buf_len, GFP_KERNEL); if (!hdr_buf || !ses_dev) goto err_init_free; page = 1; - result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE); + result = ses_recv_diag(sdev, page, hdr_buf, hdr_buf_len); if (result) goto recv_failed; len = (hdr_buf[2] << 8) + hdr_buf[3] + 4; + if (len == hdr_buf_len) + sdev_printk(KERN_WARNING, sdev, + "Suspicious page1 len %d, try larger init_alloc_size\n", len); buf = kzalloc(len, GFP_KERNEL); if (!buf) goto err_free; @@ -741,11 +770,14 @@ static int ses_intf_add(struct device *cdev) buf = NULL; page = 2; - result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE); + result = ses_recv_diag(sdev, page, hdr_buf, hdr_buf_len); if (result) goto page2_not_supported; len = (hdr_buf[2] << 8) + hdr_buf[3] + 4; + if (len == hdr_buf_len) + sdev_printk(KERN_WARNING, sdev, + "Suspicious page2 len %d, try larger init_alloc_size\n", len); buf = kzalloc(len, GFP_KERNEL); if (!buf) goto err_free; @@ -761,10 +793,13 @@ static int ses_intf_add(struct device *cdev) /* The additional information page --- allows us * to match up the devices */ page = 10; - result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE); + result = ses_recv_diag(sdev, page, hdr_buf, hdr_buf_len); if (!result) { len = (hdr_buf[2] << 8) + hdr_buf[3] + 4; + if (len == hdr_buf_len) + sdev_printk(KERN_WARNING, sdev, + "Suspicious page10 len %d, try larger init_alloc_size\n", len); buf = kzalloc(len, GFP_KERNEL); if (!buf) goto err_free;