Thread (125 messages) 125 messages, 4 authors, 2016-10-03
STALE3551d REVIEWED: 1 (0M)

[PATCH 3.12 042/119] s390/sclp_ctl: fix potential information leak with /dev/sclp

From: Jiri Slaby <hidden>
Date: 2016-09-29 10:48:07
Also in: lkml
Subsystem: s390 architecture, the rest · Maintainers: Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Linus Torvalds

From: Martin Schwidefsky <redacted>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 532c34b5fbf1687df63b3fcd5b2846312ac943c6 upstream.

The sclp_ctl_ioctl_sccb function uses two copy_from_user calls to
retrieve the sclp request from user space. The first copy_from_user
fetches the length of the request which is stored in the first two
bytes of the request. The second copy_from_user gets the complete
sclp request, but this copies the length field a second time.
A malicious user may have changed the length in the meantime.

Reported-by: Pengfei Wang <redacted>
Reviewed-by: Michael Holzheu <redacted>
Signed-off-by: Martin Schwidefsky <redacted>
Signed-off-by: Juerg Haefliger <redacted>
Signed-off-by: Jiri Slaby <redacted>
---
 drivers/s390/char/sclp_ctl.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/char/sclp_ctl.c b/drivers/s390/char/sclp_ctl.c
index 648cb86afd42..ea607a4a1bdd 100644
--- a/drivers/s390/char/sclp_ctl.c
+++ b/drivers/s390/char/sclp_ctl.c
@@ -56,6 +56,7 @@ static int sclp_ctl_ioctl_sccb(void __user *user_area)
 {
 	struct sclp_ctl_sccb ctl_sccb;
 	struct sccb_header *sccb;
+	unsigned long copied;
 	int rc;
 
 	if (copy_from_user(&ctl_sccb, user_area, sizeof(ctl_sccb)))
@@ -65,14 +66,15 @@ static int sclp_ctl_ioctl_sccb(void __user *user_area)
 	sccb = (void *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
 	if (!sccb)
 		return -ENOMEM;
-	if (copy_from_user(sccb, u64_to_uptr(ctl_sccb.sccb), sizeof(*sccb))) {
+	copied = PAGE_SIZE -
+		copy_from_user(sccb, u64_to_uptr(ctl_sccb.sccb), PAGE_SIZE);
+	if (offsetof(struct sccb_header, length) +
+	    sizeof(sccb->length) > copied || sccb->length > copied) {
 		rc = -EFAULT;
 		goto out_free;
 	}
-	if (sccb->length > PAGE_SIZE || sccb->length < 8)
-		return -EINVAL;
-	if (copy_from_user(sccb, u64_to_uptr(ctl_sccb.sccb), sccb->length)) {
-		rc = -EFAULT;
+	if (sccb->length < 8) {
+		rc = -EINVAL;
 		goto out_free;
 	}
 	rc = sclp_sync_request(ctl_sccb.cmdw, sccb);
-- 
2.10.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help