From patchwork Mon Dec 11 12:11:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 121384 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp2709935qgn; Mon, 11 Dec 2017 04:11:38 -0800 (PST) X-Google-Smtp-Source: ACJfBov+CB30FA5ke8ZCN/MDZnMumYsbmu271q/VhmLrBuxogI5Oqsu3paV0Lkrt+GJgaI54EY+n X-Received: by 10.84.169.4 with SMTP id g4mr147833plb.393.1512994298223; Mon, 11 Dec 2017 04:11:38 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1512994298; cv=none; d=google.com; s=arc-20160816; b=KRJbT3FNa6eoLj80MVrF1tFK2lQ1SAdCXqEmvL8oguI68xY+S4ypx5foLYLioLAUIy mpf0B4IDCshMmd+0398K4QIbgfMKAX6YAf5dw8kSubFkdxKdrj0zhcWDVl+jDNCsR6PG 4Zh7MlybI7WMX4PLhHFFUQfdlOv1tbnBPSZ5CI7cXLwkKrEPELYo1cCpci2CMGyFf8TR w2CTjdUSoIMlzGkMOHRGTo/Mpv9/w8AR+gHMauyzMZ87QHRRUWslblqKQfQkkmuzwzbr Ln0pK6bLgNV5POzW7E66/oopxSzRsm973wehIDbV5ev4SmMclZa//+Ij4A9pHtm+BfsO OTaQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=Mk8dqAxQq9SyKbSrdZ6judq99Pft8jljD9jk+3O58L4=; b=H5lMRLdVL38AUlZwDHdgITDom73x19ID47KZqeZJ3b03pgqHUfcG5IBFZdYP/hHdNZ tFjRXOhizpjUr+2aw61ZZrKYUutDxCufco9rCbYEhEA6cD3vFF+4ZTLQOjGwJbsAYxja RUdl6pXqdNp589nST5tBX4rjAQBL8FeUr/EmIg92KZVmKKNB09wS3pWYTI+4fN2wnHv1 PLe1ZNxIa2EyMHi7ene0x8wTlwQdBMLwoAXfesDowf0GCT3Mq+7y7G5ryggmGuNFKi3H yZ4t1wcVSH9Q1yyXqLvP/CBrj7+ICcOSqRw7YUJ8SovXkFwJgbOGaDSDeWn+7zm83EMU ivng== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l1si5904405plb.703.2017.12.11.04.11.37; Mon, 11 Dec 2017 04:11:38 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752723AbdLKMLg (ORCPT + 25 others); Mon, 11 Dec 2017 07:11:36 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:60742 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751281AbdLKMLf (ORCPT ); Mon, 11 Dec 2017 07:11:35 -0500 Received: from wuerfel.lan ([149.172.96.106]) by mrelayeu.kundenserver.de (mreue102 [212.227.15.145]) with ESMTPA (Nemesis) id 0LkyOr-1ewsCX3ODN-00amha; Mon, 11 Dec 2017 13:11:32 +0100 From: Arnd Bergmann To: Jens Axboe Cc: Arnd Bergmann , Kees Cook , linux-kernel@vger.kernel.org Subject: [PATCH] DAC960: split up ioctl function to reduce stack size Date: Mon, 11 Dec 2017 13:11:17 +0100 Message-Id: <20171211121131.3921009-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:BQuQt1iohi4zKr2H4CHX7xF8afPEgJv6rxYH49agHEacgeDoXOh 1MwUESwO0F+5oKvFtJEaRPUkUa9EUSirLgrnDXsEqDEbgsKFpmy5AhBfbU7N4tFGulgrgLV MEdghoJcpRC2OwP/M+SEWyMxghleFskpja8F6ReHMnW4cRww+wTZbNSbB1AKA/rWbv7k7Fm YP66o0QerfveZTbhRHodA== X-UI-Out-Filterresults: notjunk:1; V01:K0:9ZyhOUaPBVo=:vgaMv0fHKUPM5JYSi3WowR 6nwYq+yagrFvn85zJUDJFYT7eTQCnw87dCeO6aqPyMEA1QD2BX3eKo51khHH1kzuyferFHoBJ Da6oyNSMtvtZBkL+CkkWq+9Xf6mA6hZLC10yE2a6URbSWkFFpr+i6tkK1UWASyuoW0wMILtFv feAZQ21fjb/TO8S8RW0QpqJ6GC7wyAdlbNwxqm5POzFgS9GfppSaByymBommpCX6dojgXfJHk QdDGpg9j5iwneam5ljQ5qGIG8dE+el4+8q5xpFwRYg3WP/U/xRZUNagCDABMGV6ETsD2lxaRy 1q1LRqGyks8NIcIXeh9XPGizqkJ+TTawpTdf2+89FF7DoXuoDdIj11Qs+jJxEDPOUeUB9h5PG bFwM3UM7p7+nFW1t0F02XZkWGm/o3zQxGfb35Ufe1k9yTgYCdMUwVtg81/lasn+kIxqHNf9T6 NCRU23Drhvudr7DxikSITvG9I3OCCy5vNbwN3moIIt3iDz+XFnkiSzM4wLqlO2LvbAptBhidJ yUh0UXhJtmO58izr5SGNHFxcS4RNMuGFHIDEFYX97H7Sb2XmYH+dmBaYKvrFTXB+WOh1PBDyh 1xBdhZCQXZd+lOQXwWwJ5W/1UojqG1wMICXO3xUPAq7pV7t53VC7DofPHfSgUMXz11aY3eZMX FcVsL3tuU+2OR7rZ3PDAqyD9Q3U/ZtJ+vYIJCX+HzFYgh3+EB+ZDOfsLPrbOPRqmTj001CXDA SIPCwL7ZDM0qgWZmEJ+8Y5MtUa2A6ZJXCPTMZg== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When CONFIG_KASAN is set, all the local variables in this function are allocated on the stack together, leading to a warning about possible kernel stack overflow: drivers/block/DAC960.c: In function 'DAC960_gam_ioctl': drivers/block/DAC960.c:7061:1: error: the frame size of 2240 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] By splitting up the function into smaller chunks, we can avoid that and make the code slightly more readable at the same time. The coding style in this file is completely nonstandard, and I chose to not touch that at all, leaving the unconventional intendation unchanged to make it easier to review the diff. Signed-off-by: Arnd Bergmann --- drivers/block/DAC960.c | 160 +++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 70 deletions(-) -- 2.9.0 diff --git a/drivers/block/DAC960.c b/drivers/block/DAC960.c index 442e777bdfb2..728075214959 100644 --- a/drivers/block/DAC960.c +++ b/drivers/block/DAC960.c @@ -6619,43 +6619,27 @@ static void DAC960_DestroyProcEntries(DAC960_Controller_T *Controller) #ifdef DAC960_GAM_MINOR -/* - * DAC960_gam_ioctl is the ioctl function for performing RAID operations. -*/ - -static long DAC960_gam_ioctl(struct file *file, unsigned int Request, - unsigned long Argument) +static long DAC960_gam_get_controller_info(DAC960_ControllerInfo_T __user *UserSpaceControllerInfo) { - long ErrorCode = 0; - if (!capable(CAP_SYS_ADMIN)) return -EACCES; - - mutex_lock(&DAC960_mutex); - switch (Request) - { - case DAC960_IOCTL_GET_CONTROLLER_COUNT: - ErrorCode = DAC960_ControllerCount; - break; - case DAC960_IOCTL_GET_CONTROLLER_INFO: - { - DAC960_ControllerInfo_T __user *UserSpaceControllerInfo = - (DAC960_ControllerInfo_T __user *) Argument; DAC960_ControllerInfo_T ControllerInfo; DAC960_Controller_T *Controller; int ControllerNumber; + long ErrorCode; + if (UserSpaceControllerInfo == NULL) ErrorCode = -EINVAL; else ErrorCode = get_user(ControllerNumber, &UserSpaceControllerInfo->ControllerNumber); if (ErrorCode != 0) - break; + goto out; ErrorCode = -ENXIO; if (ControllerNumber < 0 || ControllerNumber > DAC960_ControllerCount - 1) { - break; + goto out; } Controller = DAC960_Controllers[ControllerNumber]; if (Controller == NULL) - break; + goto out; memset(&ControllerInfo, 0, sizeof(DAC960_ControllerInfo_T)); ControllerInfo.ControllerNumber = ControllerNumber; ControllerInfo.FirmwareType = Controller->FirmwareType; @@ -6670,12 +6654,12 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, strcpy(ControllerInfo.FirmwareVersion, Controller->FirmwareVersion); ErrorCode = (copy_to_user(UserSpaceControllerInfo, &ControllerInfo, sizeof(DAC960_ControllerInfo_T)) ? -EFAULT : 0); - break; - } - case DAC960_IOCTL_V1_EXECUTE_COMMAND: - { - DAC960_V1_UserCommand_T __user *UserSpaceUserCommand = - (DAC960_V1_UserCommand_T __user *) Argument; +out: + return ErrorCode; +} + +static long DAC960_gam_v1_execute_command(DAC960_V1_UserCommand_T __user *UserSpaceUserCommand) +{ DAC960_V1_UserCommand_T UserCommand; DAC960_Controller_T *Controller; DAC960_Command_T *Command = NULL; @@ -6688,39 +6672,41 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, int ControllerNumber, DataTransferLength; unsigned char *DataTransferBuffer = NULL; dma_addr_t DataTransferBufferDMA; + long ErrorCode; + if (UserSpaceUserCommand == NULL) { ErrorCode = -EINVAL; - break; + goto out; } if (copy_from_user(&UserCommand, UserSpaceUserCommand, sizeof(DAC960_V1_UserCommand_T))) { ErrorCode = -EFAULT; - break; + goto out; } ControllerNumber = UserCommand.ControllerNumber; ErrorCode = -ENXIO; if (ControllerNumber < 0 || ControllerNumber > DAC960_ControllerCount - 1) - break; + goto out; Controller = DAC960_Controllers[ControllerNumber]; if (Controller == NULL) - break; + goto out; ErrorCode = -EINVAL; if (Controller->FirmwareType != DAC960_V1_Controller) - break; + goto out; CommandOpcode = UserCommand.CommandMailbox.Common.CommandOpcode; DataTransferLength = UserCommand.DataTransferLength; if (CommandOpcode & 0x80) - break; + goto out; if (CommandOpcode == DAC960_V1_DCDB) { if (copy_from_user(&DCDB, UserCommand.DCDB, sizeof(DAC960_V1_DCDB_T))) { ErrorCode = -EFAULT; - break; + goto out; } if (DCDB.Channel >= DAC960_V1_MaxChannels) - break; + goto out; if (!((DataTransferLength == 0 && DCDB.Direction == DAC960_V1_DCDB_NoDataTransfer) || @@ -6730,15 +6716,15 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, (DataTransferLength < 0 && DCDB.Direction == DAC960_V1_DCDB_DataTransferSystemToDevice))) - break; + goto out; if (((DCDB.TransferLengthHigh4 << 16) | DCDB.TransferLength) != abs(DataTransferLength)) - break; + goto out; DCDB_IOBUF = pci_alloc_consistent(Controller->PCIDevice, sizeof(DAC960_V1_DCDB_T), &DCDB_IOBUFDMA); if (DCDB_IOBUF == NULL) { ErrorCode = -ENOMEM; - break; + goto out; } } ErrorCode = -ENOMEM; @@ -6748,19 +6734,19 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, DataTransferLength, &DataTransferBufferDMA); if (DataTransferBuffer == NULL) - break; + goto out; } else if (DataTransferLength < 0) { DataTransferBuffer = pci_alloc_consistent(Controller->PCIDevice, -DataTransferLength, &DataTransferBufferDMA); if (DataTransferBuffer == NULL) - break; + goto out; if (copy_from_user(DataTransferBuffer, UserCommand.DataTransferBuffer, -DataTransferLength)) { ErrorCode = -EFAULT; - break; + goto out; } } if (CommandOpcode == DAC960_V1_DCDB) @@ -6837,12 +6823,12 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, if (DCDB_IOBUF != NULL) pci_free_consistent(Controller->PCIDevice, sizeof(DAC960_V1_DCDB_T), DCDB_IOBUF, DCDB_IOBUFDMA); - break; - } - case DAC960_IOCTL_V2_EXECUTE_COMMAND: - { - DAC960_V2_UserCommand_T __user *UserSpaceUserCommand = - (DAC960_V2_UserCommand_T __user *) Argument; + out: + return ErrorCode; +} + +static long DAC960_gam_v2_execute_command(DAC960_V2_UserCommand_T __user *UserSpaceUserCommand) +{ DAC960_V2_UserCommand_T UserCommand; DAC960_Controller_T *Controller; DAC960_Command_T *Command = NULL; @@ -6855,26 +6841,26 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, dma_addr_t DataTransferBufferDMA; unsigned char *RequestSenseBuffer = NULL; dma_addr_t RequestSenseBufferDMA; + long ErrorCode = -EINVAL; - ErrorCode = -EINVAL; if (UserSpaceUserCommand == NULL) - break; + goto out; if (copy_from_user(&UserCommand, UserSpaceUserCommand, sizeof(DAC960_V2_UserCommand_T))) { ErrorCode = -EFAULT; - break; + goto out; } ErrorCode = -ENXIO; ControllerNumber = UserCommand.ControllerNumber; if (ControllerNumber < 0 || ControllerNumber > DAC960_ControllerCount - 1) - break; + goto out; Controller = DAC960_Controllers[ControllerNumber]; if (Controller == NULL) - break; + goto out; if (Controller->FirmwareType != DAC960_V2_Controller){ ErrorCode = -EINVAL; - break; + goto out; } DataTransferLength = UserCommand.DataTransferLength; ErrorCode = -ENOMEM; @@ -6884,14 +6870,14 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, DataTransferLength, &DataTransferBufferDMA); if (DataTransferBuffer == NULL) - break; + goto out; } else if (DataTransferLength < 0) { DataTransferBuffer = pci_alloc_consistent(Controller->PCIDevice, -DataTransferLength, &DataTransferBufferDMA); if (DataTransferBuffer == NULL) - break; + goto out; if (copy_from_user(DataTransferBuffer, UserCommand.DataTransferBuffer, -DataTransferLength)) { @@ -7001,42 +6987,44 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, if (RequestSenseBuffer != NULL) pci_free_consistent(Controller->PCIDevice, RequestSenseLength, RequestSenseBuffer, RequestSenseBufferDMA); - break; - } - case DAC960_IOCTL_V2_GET_HEALTH_STATUS: - { - DAC960_V2_GetHealthStatus_T __user *UserSpaceGetHealthStatus = - (DAC960_V2_GetHealthStatus_T __user *) Argument; +out: + return ErrorCode; +} + +static long DAC960_gam_v2_get_health_status(DAC960_V2_GetHealthStatus_T __user *UserSpaceGetHealthStatus) +{ DAC960_V2_GetHealthStatus_T GetHealthStatus; DAC960_V2_HealthStatusBuffer_T HealthStatusBuffer; DAC960_Controller_T *Controller; int ControllerNumber; + long ErrorCode; + if (UserSpaceGetHealthStatus == NULL) { ErrorCode = -EINVAL; - break; + goto out; } if (copy_from_user(&GetHealthStatus, UserSpaceGetHealthStatus, sizeof(DAC960_V2_GetHealthStatus_T))) { ErrorCode = -EFAULT; - break; + goto out; } ErrorCode = -ENXIO; ControllerNumber = GetHealthStatus.ControllerNumber; if (ControllerNumber < 0 || ControllerNumber > DAC960_ControllerCount - 1) - break; + goto out; Controller = DAC960_Controllers[ControllerNumber]; if (Controller == NULL) - break; + goto out; if (Controller->FirmwareType != DAC960_V2_Controller) { ErrorCode = -EINVAL; - break; + goto out; } if (copy_from_user(&HealthStatusBuffer, GetHealthStatus.HealthStatusBuffer, sizeof(DAC960_V2_HealthStatusBuffer_T))) { ErrorCode = -EFAULT; - break; + goto out; } ErrorCode = wait_event_interruptible_timeout(Controller->HealthStatusWaitQueue, !(Controller->V2.HealthStatusBuffer->StatusChangeCounter @@ -7046,7 +7034,7 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, DAC960_MonitoringTimerInterval); if (ErrorCode == -ERESTARTSYS) { ErrorCode = -EINTR; - break; + goto out; } if (copy_to_user(GetHealthStatus.HealthStatusBuffer, Controller->V2.HealthStatusBuffer, @@ -7054,7 +7042,39 @@ static long DAC960_gam_ioctl(struct file *file, unsigned int Request, ErrorCode = -EFAULT; else ErrorCode = 0; - } + +out: + return ErrorCode; +} + +/* + * DAC960_gam_ioctl is the ioctl function for performing RAID operations. +*/ + +static long DAC960_gam_ioctl(struct file *file, unsigned int Request, + unsigned long Argument) +{ + long ErrorCode = 0; + void __user *argp = (void __user *)Argument; + if (!capable(CAP_SYS_ADMIN)) return -EACCES; + + mutex_lock(&DAC960_mutex); + switch (Request) + { + case DAC960_IOCTL_GET_CONTROLLER_COUNT: + ErrorCode = DAC960_ControllerCount; + break; + case DAC960_IOCTL_GET_CONTROLLER_INFO: + ErrorCode = DAC960_gam_get_controller_info(argp); + break; + case DAC960_IOCTL_V1_EXECUTE_COMMAND: + ErrorCode = DAC960_gam_v1_execute_command(argp); + break; + case DAC960_IOCTL_V2_EXECUTE_COMMAND: + ErrorCode = DAC960_gam_v2_execute_command(argp); + break; + case DAC960_IOCTL_V2_GET_HEALTH_STATUS: + ErrorCode = DAC960_gam_v2_get_health_status(argp); break; default: ErrorCode = -ENOTTY;