linux-kernel - [PATCH v3] drivers/vfio: Fix a redundant copy bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 17 Oct 2018 09:32:04 -0500
From:   Wenwen Wang <wang6495@....edu>
To:     Wenwen Wang <wang6495@....edu>
Cc:     Kangjie Lu <kjlu@....edu>,
        Alex Williamson <alex.williamson@...hat.com>,
        kvm@...r.kernel.org (open list:VFIO DRIVER),
        linux-kernel@...r.kernel.org (open list)
Subject: [PATCH v3] drivers/vfio: Fix a redundant copy bug

In vfio_spapr_iommu_eeh_ioctl(), if the ioctl command is VFIO_EEH_PE_OP,
the user-space buffer 'arg' is copied to the kernel object 'op' and the
'argsz' and 'flags' fields of 'op' are checked. If the check fails, an
error code EINVAL is returned. Otherwise, 'op.op' is further checked
through a switch statement to invoke related handlers. If 'op.op' is
VFIO_EEH_PE_INJECT_ERR, the whole user-space buffer 'arg' is copied again
to 'op' to obtain the err information. However, in the following execution
of this case, the fields of 'op', except the field 'err', are actually not
used. That is, the second copy has a redundant part. Therefore, for both
performance consideration, the redundant part of the second copy should be
removed.

This patch removes such a part in the second copy. It only copies from
'err.type' to 'err.mask', which is exactly required by the
VFIO_EEH_PE_INJECT_ERR op.

This patch also adds a 4-byte reserved field in the structure
vfio_eeh_pe_op to make sure that the u64 fields in the structure
vfio_eeh_pe_err are 8-byte aligned.

Signed-off-by: Wenwen Wang <wang6495@....edu>
---
 drivers/vfio/vfio_spapr_eeh.c | 9 ++++++---
 include/uapi/linux/vfio.h     | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
index 38edeb4..66634c6 100644
--- a/drivers/vfio/vfio_spapr_eeh.c
+++ b/drivers/vfio/vfio_spapr_eeh.c
@@ -37,6 +37,7 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 	struct eeh_pe *pe;
 	struct vfio_eeh_pe_op op;
 	unsigned long minsz;
+	unsigned long start, end;
 	long ret = -EINVAL;

 	switch (cmd) {
@@ -86,10 +87,12 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 			ret = eeh_pe_configure(pe);
 			break;
 		case VFIO_EEH_PE_INJECT_ERR:
-			minsz = offsetofend(struct vfio_eeh_pe_op, err.mask);
-			if (op.argsz < minsz)
+			start = offsetof(struct vfio_eeh_pe_op, err.type);
+			end = offsetofend(struct vfio_eeh_pe_op, err.mask);
+			if (op.argsz < end)
 				return -EINVAL;
-			if (copy_from_user(&op, (void __user *)arg, minsz))
+			if (copy_from_user(&op.err, (char __user *)arg +
+						start, end - start))
 				return -EFAULT;

 			ret = eeh_pe_inject_err(pe, op.err.type, op.err.func,
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 1aa7b82..d904c42 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -729,6 +729,7 @@ struct vfio_eeh_pe_op {
 	__u32 argsz;
 	__u32 flags;
 	__u32 op;
+	__u32 __resv;
 	union {
 		struct vfio_eeh_pe_err err;
 	};
-- 
2.7.4