Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964908AbXBLNYd (ORCPT ); Mon, 12 Feb 2007 08:24:33 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S964910AbXBLNYd (ORCPT ); Mon, 12 Feb 2007 08:24:33 -0500 Received: from ug-out-1314.google.com ([66.249.92.168]:15311 "EHLO ug-out-1314.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964908AbXBLNYc (ORCPT ); Mon, 12 Feb 2007 08:24:32 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=KjWmMVbIAp9eYHPCxT/t4IwifEwLDAWIA+OLxY+7RAfofhilNsnFLOEUfU6EZ/hCPWzx+dX+OoPRrEvLny1L7lto00tgEhna4Hu3+E9MK1WOBzq+K1jN9cjyFCVhEqqjbDPJjFGEA1ohv7aX8AQ97M9sJMdgKcR51S5wF6An92E= Message-ID: <5c8016cf0702120524p4bac240du6dc778745f537c9d@mail.gmail.com> Date: Mon, 12 Feb 2007 13:24:29 +0000 From: "Giuliano Procida" To: "Andi Kleen" Subject: Re: [PATCH x86 for review II] [26/39] i386: fix 32-bit ioctls on x64_32 Cc: patches@x86-64.org, linux-kernel@vger.kernel.org In-Reply-To: <20070212073814.1879213DA0@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20070212837.963446000@suse.de> <20070212073814.1879213DA0@wotan.suse.de> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8401 Lines: 283 This is a nicer version of the MTRR compatibilty ioctl patch, compiles smaller and also tested. Signed-off-by: Giuliano Procida --- linux-source-2.6.19.1.orig/arch/i386/kernel/cpu/mtrr/if.c 2006-12-11 19:32:53.000000000 +0000 +++ linux-source-2.6.19.1/arch/i386/kernel/cpu/mtrr/if.c 2007-01-27 12:25:21.000000000 +0000 @@ -154,150 +154,164 @@ mtrr_ioctl(struct file *file, unsigned int cmd, unsigned long __arg) { int err = 0; + const unsigned ioctl_type = _IOC_TYPE(cmd); + const unsigned ioctl_dir = _IOC_DIR(cmd); + const unsigned ioctl_nr = _IOC_NR(cmd); + const unsigned ioctl_size = _IOC_SIZE(cmd); mtrr_type type; - struct mtrr_sentry sentry; - struct mtrr_gentry gentry; + union mtrr_data { + struct mtrr_sentry sentry; + struct mtrr_gentry gentry; +#ifdef CONFIG_COMPAT + struct mtrr_sentry32 sentry32; + struct mtrr_gentry32 gentry32; +#endif + } u; void __user *arg = (void __user *) __arg; - switch (cmd) { - case MTRRIOC_ADD_ENTRY: - case MTRRIOC_SET_ENTRY: - case MTRRIOC_DEL_ENTRY: - case MTRRIOC_KILL_ENTRY: - case MTRRIOC_ADD_PAGE_ENTRY: - case MTRRIOC_SET_PAGE_ENTRY: - case MTRRIOC_DEL_PAGE_ENTRY: - case MTRRIOC_KILL_PAGE_ENTRY: - if (copy_from_user(&sentry, arg, sizeof sentry)) - return -EFAULT; - break; - case MTRRIOC_GET_ENTRY: - case MTRRIOC_GET_PAGE_ENTRY: - if (copy_from_user(&gentry, arg, sizeof gentry)) - return -EFAULT; + /* check type and max size */ + if (ioctl_type != MTRR_IOCTL_BASE || ioctl_size > sizeof(u)) + return -ENOTTY; + + /* copy from user */ + if (ioctl_dir & _IOC_WRITE && copy_from_user(&u, arg, ioctl_size)) + return -EFAULT; + + /* check number, direction, size and permission */ + switch (ioctl_nr) { + case _IOC_NR(MTRRIOC_ADD_ENTRY): + case _IOC_NR(MTRRIOC_SET_ENTRY): + case _IOC_NR(MTRRIOC_DEL_ENTRY): + case _IOC_NR(MTRRIOC_KILL_ENTRY): + case _IOC_NR(MTRRIOC_ADD_PAGE_ENTRY): + case _IOC_NR(MTRRIOC_SET_PAGE_ENTRY): + case _IOC_NR(MTRRIOC_DEL_PAGE_ENTRY): + case _IOC_NR(MTRRIOC_KILL_PAGE_ENTRY): + if (ioctl_dir != _IOC_WRITE) + return -ENOTTY; + switch (ioctl_size) { + case sizeof(struct mtrr_sentry): break; #ifdef CONFIG_COMPAT - case MTRRIOC32_ADD_ENTRY: - case MTRRIOC32_SET_ENTRY: - case MTRRIOC32_DEL_ENTRY: - case MTRRIOC32_KILL_ENTRY: - case MTRRIOC32_ADD_PAGE_ENTRY: - case MTRRIOC32_SET_PAGE_ENTRY: - case MTRRIOC32_DEL_PAGE_ENTRY: - case MTRRIOC32_KILL_PAGE_ENTRY: { - struct mtrr_sentry32 __user *s32 = (struct mtrr_sentry32 __user *)__arg; - err = get_user(sentry.base, &s32->base); - err |= get_user(sentry.size, &s32->size); - err |= get_user(sentry.type, &s32->type); - if (err) - return err; - break; - } - case MTRRIOC32_GET_ENTRY: - case MTRRIOC32_GET_PAGE_ENTRY: { - struct mtrr_gentry32 __user *g32 = (struct mtrr_gentry32 __user *)__arg; - err = get_user(gentry.regnum, &g32->regnum); - err |= get_user(gentry.base, &g32->base); - err |= get_user(gentry.size, &g32->size); - err |= get_user(gentry.type, &g32->type); - if (err) - return err; + case sizeof(struct mtrr_sentry32): + { + struct mtrr_sentry32 s32 = u.sentry32; + u.sentry.base = s32.base; + u.sentry.size = s32.size; + u.sentry.type = s32.type; + } break; - } #endif + default: + return -ENOTTY; + } + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + break; + case _IOC_NR(MTRRIOC_GET_ENTRY): + case _IOC_NR(MTRRIOC_GET_PAGE_ENTRY): + if (ioctl_dir != (_IOC_READ|_IOC_WRITE)) + return -ENOTTY; + switch (ioctl_size) { + case sizeof(struct mtrr_gentry): + break; +#ifdef CONFIG_COMPAT + case sizeof(struct mtrr_gentry32): + { + struct mtrr_gentry32 g32 = u.gentry32; + u.gentry.base = g32.base; + u.gentry.size = g32.size; + u.gentry.regnum = g32.regnum; + u.gentry.type = g32.type; + } + break; +#endif + default: + return -ENOTTY; + } + break; + default: + return -ENOTTY; } - switch (cmd) { + /* perform command */ + switch (ioctl_nr) { default: return -ENOTTY; - case MTRRIOC_ADD_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; + case _IOC_NR(MTRRIOC_ADD_ENTRY): err = - mtrr_file_add(sentry.base, sentry.size, sentry.type, 1, + mtrr_file_add(u.sentry.base, u.sentry.size, u.sentry.type, 1, file, 0); break; - case MTRRIOC_SET_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_add(sentry.base, sentry.size, sentry.type, 0); + case _IOC_NR(MTRRIOC_SET_ENTRY): + err = mtrr_add(u.sentry.base, u.sentry.size, u.sentry.type, 0); break; - case MTRRIOC_DEL_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_file_del(sentry.base, sentry.size, file, 0); + case _IOC_NR(MTRRIOC_DEL_ENTRY): + err = mtrr_file_del(u.sentry.base, u.sentry.size, file, 0); break; - case MTRRIOC_KILL_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_del(-1, sentry.base, sentry.size); + case _IOC_NR(MTRRIOC_KILL_ENTRY): + err = mtrr_del(-1, u.sentry.base, u.sentry.size); break; - case MTRRIOC_GET_ENTRY: - if (gentry.regnum >= num_var_ranges) + case _IOC_NR(MTRRIOC_GET_ENTRY): + if (u.gentry.regnum >= num_var_ranges) return -EINVAL; - mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type); + mtrr_if->get(u.gentry.regnum, &u.gentry.base, &u.gentry.size, &type); /* Hide entries that go above 4GB */ - if (gentry.base + gentry.size > 0x100000 - || gentry.size == 0x100000) - gentry.base = gentry.size = gentry.type = 0; + if (u.gentry.base + u.gentry.size > 0x100000 + || u.gentry.size == 0x100000) + u.gentry.base = u.gentry.size = u.gentry.type = 0; else { - gentry.base <<= PAGE_SHIFT; - gentry.size <<= PAGE_SHIFT; - gentry.type = type; + u.gentry.base <<= PAGE_SHIFT; + u.gentry.size <<= PAGE_SHIFT; + u.gentry.type = type; } break; - case MTRRIOC_ADD_PAGE_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; + case _IOC_NR(MTRRIOC_ADD_PAGE_ENTRY): err = - mtrr_file_add(sentry.base, sentry.size, sentry.type, 1, + mtrr_file_add(u.sentry.base, u.sentry.size, u.sentry.type, 1, file, 1); break; - case MTRRIOC_SET_PAGE_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_add_page(sentry.base, sentry.size, sentry.type, 0); + case _IOC_NR(MTRRIOC_SET_PAGE_ENTRY): + err = mtrr_add_page(u.sentry.base, u.sentry.size, u.sentry.type, 0); break; - case MTRRIOC_DEL_PAGE_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_file_del(sentry.base, sentry.size, file, 1); + case _IOC_NR(MTRRIOC_DEL_PAGE_ENTRY): + err = mtrr_file_del(u.sentry.base, u.sentry.size, file, 1); break; - case MTRRIOC_KILL_PAGE_ENTRY: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - err = mtrr_del_page(-1, sentry.base, sentry.size); + case _IOC_NR(MTRRIOC_KILL_PAGE_ENTRY): + err = mtrr_del_page(-1, u.sentry.base, u.sentry.size); break; - case MTRRIOC_GET_PAGE_ENTRY: - if (gentry.regnum >= num_var_ranges) + case _IOC_NR(MTRRIOC_GET_PAGE_ENTRY): + if (u.gentry.regnum >= num_var_ranges) return -EINVAL; - mtrr_if->get(gentry.regnum, &gentry.base, &gentry.size, &type); - gentry.type = type; + mtrr_if->get(u.gentry.regnum, &u.gentry.base, &u.gentry.size, &type); + u.gentry.type = type; break; } if (err) return err; - switch(cmd) { - case MTRRIOC_GET_ENTRY: - case MTRRIOC_GET_PAGE_ENTRY: - if (copy_to_user(arg, &gentry, sizeof gentry)) - err = -EFAULT; - break; + /* copy to user */ + if (ioctl_dir & _IOC_READ) { + switch (ioctl_size) { #ifdef CONFIG_COMPAT - case MTRRIOC32_GET_ENTRY: - case MTRRIOC32_GET_PAGE_ENTRY: { - struct mtrr_gentry32 __user *g32 = (struct mtrr_gentry32 __user *)__arg; - err = put_user(gentry.base, &g32->base); - err |= put_user(gentry.size, &g32->size); - err |= put_user(gentry.regnum, &g32->regnum); - err |= put_user(gentry.type, &g32->type); + case sizeof(struct mtrr_gentry32): + { + struct mtrr_gentry g64 = u.gentry; + u.gentry32.base = g64.base; + u.gentry32.size = g64.size; + u.gentry32.regnum = g64.regnum; + u.gentry32.type = g64.type; + } break; - } #endif + default: + break; + } + if (copy_to_user(arg, &u, ioctl_size)) + err = -EFAULT; } return err; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/