[PATCH] scsi: sg, sr: fix stale scatter_elem_sz in sg_build_indirect() and OOB read in sr_is_xa()

Hui Peng posted 1 patch 4 days, 22 hours ago
[PATCH] scsi: sg, sr: fix stale scatter_elem_sz in sg_build_indirect() and OOB read in sr_is_xa()
Posted by Hui Peng 4 days, 22 hours ago
Fix two issues in drivers/scsi/sg.c and drivers/scsi/sr_ioctl.c:

1. In sg_build_indirect() (drivers/scsi/sg.c), snapshot scatter_elem_sz
   into a local variable and clamp it to [PAGE_SIZE, SG_SCATTER_SZ] so a
   concurrent SG_SET_RESERVED_SIZE ioctl cannot cause inconsistent page
   order calculations or unbounded order-10 allocations.
2. In sr_is_xa() (drivers/scsi/sr_ioctl.c), allocate at least 2048 bytes
   (or CD_FRAMESIZE_RAW) before calling sr_read_sector() so reading
   sector data at offset + 14 does not read or write past the kmalloc
   buffer.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5408f002e6c0..5d635349e06e 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -480,8 +480,10 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 
 	hp = &srp->header;
 	old_hdr = kzalloc(SZ_SG_HEADER, GFP_KERNEL);
-	if (!old_hdr)
-		return -ENOMEM;
+	if (!old_hdr) {
+		retval = -ENOMEM;
+		goto free_old_hdr;
+	}
 
 	old_hdr->reply_len = (int) hp->timeout;
 	old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */
@@ -543,10 +545,10 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 		}
 	} else
 		count = (old_hdr->result == 0) ? 0 : -EIO;
-	sg_finish_rem_req(srp);
-	sg_remove_request(sfp, srp);
 	retval = count;
 free_old_hdr:
+	sg_finish_rem_req(srp);
+	sg_remove_request(sfp, srp);
 	kfree(old_hdr);
 	return retval;
 }
@@ -1667,9 +1669,12 @@ init_sg(void)
 {
 	int rc;
 
-	if (scatter_elem_sz < PAGE_SIZE) {
+	if (scatter_elem_sz < (int)PAGE_SIZE) {
 		scatter_elem_sz = PAGE_SIZE;
 		scatter_elem_sz_prev = scatter_elem_sz;
+	} else if (scatter_elem_sz > (int)(PAGE_SIZE << MAX_PAGE_ORDER)) {
+		scatter_elem_sz = PAGE_SIZE << MAX_PAGE_ORDER;
+		scatter_elem_sz_prev = scatter_elem_sz;
 	}
 
 	rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), 
@@ -1875,9 +1880,14 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
 
 	num = scatter_elem_sz;
 	if (unlikely(num != scatter_elem_sz_prev)) {
-		if (num < PAGE_SIZE) {
+		if (num < (int)PAGE_SIZE) {
+			num = PAGE_SIZE;
 			scatter_elem_sz = PAGE_SIZE;
 			scatter_elem_sz_prev = PAGE_SIZE;
+		} else if (num > (int)(PAGE_SIZE << MAX_PAGE_ORDER)) {
+			num = PAGE_SIZE << MAX_PAGE_ORDER;
+			scatter_elem_sz = PAGE_SIZE << MAX_PAGE_ORDER;
+			scatter_elem_sz_prev = PAGE_SIZE << MAX_PAGE_ORDER;
 		} else
 			scatter_elem_sz_prev = num;
 	}
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 089653018d32..2a3ce6a460ed 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -582,7 +582,7 @@ int sr_is_xa(Scsi_CD *cd)
 	if (!xa_test)
 		return 0;
 
-	raw_sector = kmalloc(2048, GFP_KERNEL);
+	raw_sector = kmalloc(CD_FRAMESIZE_RAW1, GFP_KERNEL);
 	if (!raw_sector)
 		return -ENOMEM;
 	if (0 == sr_read_sector(cd, cd->ms_offset + 16,