From patchwork Thu Feb 11 13:16:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 102813 Delivered-To: patch@linaro.org Received: by 10.112.43.199 with SMTP id y7csp185345lbl; Thu, 11 Feb 2016 05:17:26 -0800 (PST) X-Received: by 10.66.156.195 with SMTP id wg3mr65805600pab.54.1455196646687; Thu, 11 Feb 2016 05:17:26 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id bs10si12597285pad.73.2016.02.11.05.17.26; Thu, 11 Feb 2016 05:17:26 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of stable-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 stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751875AbcBKNRZ (ORCPT + 3 others); Thu, 11 Feb 2016 08:17:25 -0500 Received: from mout.kundenserver.de ([212.227.126.135]:62118 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751099AbcBKNRY (ORCPT ); Thu, 11 Feb 2016 08:17:24 -0500 Received: from wuerfel.lan. ([78.42.132.4]) by mrelayeu.kundenserver.de (mreue003) with ESMTPA (Nemesis) id 0M944P-1aI3nP0YRR-00CROO; Thu, 11 Feb 2016 14:16:46 +0100 From: Arnd Bergmann To: Tejun Heo Cc: linux-arm-kernel@lists.infradead.org, Arnd Bergmann , Soohoon Lee , stable@vger.kernel.org, linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] libata: fix HDIO_GET_32BIT ioctl Date: Thu, 11 Feb 2016 14:16:27 +0100 Message-Id: <1455196595-3466120-1-git-send-email-arnd@arndb.de> X-Mailer: git-send-email 2.7.0 X-Provags-ID: V03:K0:ZGRWt6AHoH0tHUwPIbqgZ3tQ9sUazBC4Y3dpkYsETG9GYu/aWcN p2oyYlqJB7l12pRW/0Nc2lZDcIIc/saVkY8uM1hvY4KHX7xgbFUH7xVDYfKMFyENeXTz0YR Ut2310svIILOXfikOxJUkKlQgUNPt2d1H9fgw5dxWyXnM4qQ2lonScD2Aw5OJQTS0GXyp6c 7LX2n31HMk2H4Vk+sCW3Q== X-UI-Out-Filterresults: notjunk:1; V01:K0:aXW7MRLQJlY=:HrO7WGl548zuisJx33luLI NjIiQkxdevuT7PEolBaiYoYvh0fxgLuhyHlum7n9YmXYKcAk2yfqL7o1+zEsKeGyaXiEcR5Y6 hno1K4KV7tEOzSS3/criOe5ZfgU81q6/5FeV58C4RPbVXeTxeAi5bk7MbILQQtpo+IsHUrsHT ZMuTVMQTonCmnpiXSLiio6hC8NYXycNqiVEbXtgFqVARtwHJ7mwPvnGqHhSdm+7kOur9Jv9xf 4zKltWdgrzPQkQiNEhhp0ifoq/9gSmTaSiDicqqW6BWYkvy1KveOsiJtM1ezuVN9cWlHhJa9I LcUjLvGlpgPUEB4P14aktGnt8x3CJwd2UynRKLcbsbQy/DfAZzwYp0Ye24Wv4NOZ12yetfgqK xAEmXfiFdfjvYdA8H3bDPkIb8owIt/TSvjAW5qoELbDZkV/ZAaWd74oE7w9aZjGOBrF2Oa8fT /wg/EsDNqNh+sx1tiImfUjI5QBHeT0amdyO+4t1XxTZtcxszVL6jYyrOKZpX0yJTjE0gTkqsx arxZLQeBnvbjEXSbAO5mU5LkIcf7b58CpYMC4/AThipOmmBxd8C/YMOSoowT8QnW9XE9cThq2 ZnP0DpeT99yr32VIegBKyzRkMLhRXEtrqCv755tVR2Mufk8DQfqbIDfdv7hxI8jm3r7v7s0jY PBUf1Ltk4m9yhUCvs3Xx666SmbdCdpy0hvZ20BHRNVmJSJRcen6ZbJBdMTOrwAcIkMr0= Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org As reported by Soohoon Lee, the HDIO_GET_32BIT ioctl does not work correctly in compat mode with libata. I have investigated the issue further and found multiple problems that all appeared with the same commit that originally introduced HDIO_GET_32BIT handling in libata back in linux-2.6.8 and presumably also linux-2.4, as the code uses "copy_to_user(arg, &val, 1)" to copy a 'long' variable containing either 0 or 1 to user space. The problems with this are: * On big-endian machines, this will always write a zero because it stores the wrong byte into user space. * In compat mode, the upper three bytes of the variable are updated by the compat_hdio_ioctl() function, but they now contain uninitialized stack data. * The hdparm tool calling this ioctl uses a 'static long' variable to store the result. This means at least the upper bytes are initialized to zero, but calling another ioctl like HDIO_GET_MULTCOUNT would fill them with data that remains stale when the low byte is overwritten. Fortunately libata doesn't implement any of the affected ioctl commands, so this would only happen when we query both an IDE and an ATA device in the same command such as "hdparm -N -c /dev/hda /dev/sda" * The libata code for unknown reasons started using ATA_IOC_GET_IO32 and ATA_IOC_SET_IO32 as aliases for HDIO_GET_32BIT and HDIO_SET_32BIT, while the ioctl commands that were added later use the normal HDIO_* names. This is harmless but rather confusing. This addresses all four issues by changing the code to use put_user() on an 'unsigned long' variable in HDIO_GET_32BIT, like the IDE subsystem does, and by clarifying the names of the ioctl commands. Signed-off-by: Arnd Bergmann Reported-by: Soohoon Lee Tested-by: Soohoon Lee Cc: stable@vger.kernel.org --- drivers/ata/libata-scsi.c | 11 +++++------ include/linux/ata.h | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 7e959f90c020..e417e1a1d02c 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -675,19 +675,18 @@ static int ata_ioc32(struct ata_port *ap) int ata_sas_scsi_ioctl(struct ata_port *ap, struct scsi_device *scsidev, int cmd, void __user *arg) { - int val = -EINVAL, rc = -EINVAL; + unsigned long val; + int rc = -EINVAL; unsigned long flags; switch (cmd) { - case ATA_IOC_GET_IO32: + case HDIO_GET_32BIT: spin_lock_irqsave(ap->lock, flags); val = ata_ioc32(ap); spin_unlock_irqrestore(ap->lock, flags); - if (copy_to_user(arg, &val, 1)) - return -EFAULT; - return 0; + return put_user(val, (unsigned long __user *)arg); - case ATA_IOC_SET_IO32: + case HDIO_SET_32BIT: val = (unsigned long) arg; rc = 0; spin_lock_irqsave(ap->lock, flags); diff --git a/include/linux/ata.h b/include/linux/ata.h index d2992bfa1706..c1a2f345cbe6 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -487,8 +487,8 @@ enum ata_tf_protocols { }; enum ata_ioctls { - ATA_IOC_GET_IO32 = 0x309, - ATA_IOC_SET_IO32 = 0x324, + ATA_IOC_GET_IO32 = 0x309, /* HDIO_GET_32BIT */ + ATA_IOC_SET_IO32 = 0x324, /* HDIO_SET_32BIT */ }; /* core structures */