From patchwork Tue Oct 21 11:50:25 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Fleming X-Patchwork-Id: 401452 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id DD99A14001A; Tue, 21 Oct 2014 22:50:51 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1XgXxi-0007Pp-QK; Tue, 21 Oct 2014 11:50:50 +0000 Received: from mail-wi0-f175.google.com ([209.85.212.175]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1XgXxS-0007Ne-5N for fwts-devel@lists.ubuntu.com; Tue, 21 Oct 2014 11:50:34 +0000 Received: by mail-wi0-f175.google.com with SMTP id d1so9863694wiv.2 for ; Tue, 21 Oct 2014 04:50:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=s4zkrmOQvR0tJy+u/0xvPswgCZvZPG4Fihb66rdbir4=; b=TSx6c6hZa64inrdvvsoP15NU1ehTAeqBvX6xb5K/W9CIHx16SwAjpesyHlexwlpsTu T4cB11T3YWjEiUBeGV8AqHlNnI+0j/4x/0Tiy8tgFg2I8hIPclZXko4lOamQ1XsLJugL I/NX2K92amfXobeEOCJkgTjNmg1kTuwcAYw4C2JRHmhbOx0Ku+4dBcMyoXCqq5iKkgGI PJ5zyjwPkUaeRgQM9VYcjd85UNc9u3FHMLQa3/65mdx19xjpRGfLOhUw5SSprn7ynjn1 OKgs+VAFNvyyEVW4lJKjJZe/lQzFYRou4dU2wBSfqX7q/2itq5qAlpFhnTdF0hHAtv7q 4UaQ== X-Gm-Message-State: ALoCoQk+gMM8h9bO2hUvQ6ayFK6kSB9y624/8yb2eRXsKC9AfI6L+b+VBDlx6YBSmrQkyJ/wT5f5 X-Received: by 10.194.239.164 with SMTP id vt4mr3589190wjc.131.1413892233207; Tue, 21 Oct 2014 04:50:33 -0700 (PDT) Received: from localhost ([94.8.52.96]) by mx.google.com with ESMTPSA id fa3sm274477wid.0.2014.10.21.04.50.32 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Oct 2014 04:50:32 -0700 (PDT) From: Matt Fleming To: fwts-devel@lists.ubuntu.com Subject: [PATCH 1/2] efi_runtime: Copied the structure from userland locally in kernel space Date: Tue, 21 Oct 2014 12:50:25 +0100 Message-Id: <1413892226-27849-2-git-send-email-matt@console-pimps.org> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1413892226-27849-1-git-send-email-matt@console-pimps.org> References: <1413892226-27849-1-git-send-email-matt@console-pimps.org> Cc: Pradeep Gaddam X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: fwts-devel-bounces@lists.ubuntu.com Sender: fwts-devel-bounces@lists.ubuntu.com From: Pradeep Gaddam When we have structures that contain pointers, we cannot just do double dereference on it as in struct->pointer->value. This will end up as a Page Fault. To fix this problem, I changed efi_runtime kernel module to first fetch the entire structure into a local copy on stack and use that to reference the other data members of the struct. Signed-off-by: Pradeep Gaddam Acked-by: Alex Hung Acked-by: Ivan Hu --- efi_runtime/efi_runtime.c | 50 ++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c index 4f9d5545f2f5..59609bc956b2 100644 --- a/efi_runtime/efi_runtime.c +++ b/efi_runtime/efi_runtime.c @@ -301,44 +301,51 @@ static long efi_runtime_set_variable(unsigned long arg) static long efi_runtime_get_time(unsigned long arg) { struct efi_gettime __user *pgettime; + struct efi_gettime pgettime_local; + EFI_TIME_CAPABILITIES __user *cap_local; efi_status_t status; efi_time_cap_t cap; efi_time_t eft; status = efi.get_time(&eft, &cap); pgettime = (struct efi_gettime __user *)arg; - if (put_user(status, pgettime->status)) + if (copy_from_user(&pgettime_local, pgettime, sizeof(pgettime_local))) + return -EFAULT; + + cap_local = (EFI_TIME_CAPABILITIES *)pgettime_local.Capabilities; + if (put_user(status, pgettime_local.status)) return -EFAULT; if (status != EFI_SUCCESS) { printk(KERN_ERR "efitime: can't read time\n"); return -EINVAL; } if (put_user(cap.resolution, - &pgettime->Capabilities->Resolution) || - put_user(cap.accuracy, - &pgettime->Capabilities->Accuracy) || - put_user(cap.sets_to_zero, - &pgettime->Capabilities->SetsToZero)) + &(cap_local->Resolution)) || + put_user(cap.accuracy, &(cap_local->Accuracy)) || + put_user(cap.sets_to_zero,&(cap_local->SetsToZero))) return -EFAULT; - return copy_to_user(pgettime->Time, &eft, + return copy_to_user(pgettime_local.Time, &eft, sizeof(EFI_TIME)) ? -EFAULT : 0; } static long efi_runtime_set_time(unsigned long arg) { struct efi_settime __user *psettime; + struct efi_settime psettime_local; efi_status_t status; EFI_TIME efi_time; efi_time_t eft; psettime = (struct efi_settime __user *)arg; - if (copy_from_user(&efi_time, psettime->Time, + if (copy_from_user(&psettime_local, psettime, sizeof(psettime_local))) + return -EFAULT; + if (copy_from_user(&efi_time, psettime_local.Time, sizeof(EFI_TIME))) return -EFAULT; convert_to_efi_time(&eft, &efi_time); status = efi.set_time(&eft); - if (put_user(status, psettime->status)) + if (put_user(status, psettime_local.status)) return -EFAULT; return status == EFI_SUCCESS ? 0 : -EINVAL; @@ -347,6 +354,7 @@ static long efi_runtime_set_time(unsigned long arg) static long efi_runtime_get_waketime(unsigned long arg) { struct efi_getwakeuptime __user *pgetwakeuptime; + struct efi_getwakeuptime pgetwakeuptime_local; unsigned char enabled, pending; efi_status_t status; EFI_TIME efi_time; @@ -357,24 +365,25 @@ static long efi_runtime_get_waketime(unsigned long arg) pgetwakeuptime = (struct efi_getwakeuptime __user *)arg; - if (put_user(status, pgetwakeuptime->status)) + if (copy_from_user(&pgetwakeuptime_local, pgetwakeuptime, sizeof(pgetwakeuptime_local))) + return -EFAULT; + if (put_user(status, pgetwakeuptime_local.status)) return -EFAULT; if (status != EFI_SUCCESS) return -EINVAL; - - if (put_user(enabled, pgetwakeuptime->Enabled) || - put_user(pending, pgetwakeuptime->Pending)) + if (put_user(enabled, pgetwakeuptime_local.Enabled) || + put_user(pending, pgetwakeuptime_local.Pending)) return -EFAULT; - convert_from_efi_time(&eft, &efi_time); - return copy_to_user(pgetwakeuptime->Time, &efi_time, + return copy_to_user(pgetwakeuptime_local.Time, &efi_time, sizeof(EFI_TIME)) ? -EFAULT : 0; } static long efi_runtime_set_waketime(unsigned long arg) { struct efi_setwakeuptime __user *psetwakeuptime; + struct efi_setwakeuptime psetwakeuptime_local; unsigned char enabled; efi_status_t status; EFI_TIME efi_time; @@ -382,17 +391,18 @@ static long efi_runtime_set_waketime(unsigned long arg) psetwakeuptime = (struct efi_setwakeuptime __user *)arg; - if (get_user(enabled, &psetwakeuptime->Enabled) || - copy_from_user(&efi_time, - psetwakeuptime->Time, - sizeof(EFI_TIME))) + if (copy_from_user(&psetwakeuptime_local, psetwakeuptime, sizeof(psetwakeuptime_local))) + return -EFAULT; + + if (get_user(enabled, &(psetwakeuptime_local.Enabled)) || + copy_from_user(&efi_time, psetwakeuptime_local.Time, sizeof(EFI_TIME))) return -EFAULT; convert_to_efi_time(&eft, &efi_time); status = efi.set_wakeup_time(enabled, &eft); - if (put_user(status, psetwakeuptime->status)) + if (put_user(status, psetwakeuptime_local.status)) return -EFAULT; return status == EFI_SUCCESS ? 0 : -EINVAL;