[CHECKER] aacraid user pointer use

[CHECKER] aacraid user pointer use

[Hollis Blanchard]

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

Stanford checker said:
---------------------------------------------------------
[BUG] at least bad programming practice. file_opetations.ioctl ->
aac_cfg_ioctl -> aac_do_ioctl -> close_getadapter_fib ->
aac_close_fib_context. All other functions called by aac_do_ioctl assume
arg is a user pointer. It is unknown what will happen.

/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/ 
commctrl.c:277:aac_close_fib_context:
ERROR:TAINTED:277:277: dereferencing tainted ptr 'fibctx' [Callstack:
/home/junfeng/linux-2.5.63/drivers/scsi/sg.c:1002:aac_cfg_ioctl((tainted
3)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/ 
linit.c:671:aac_do_ioctl((tainted
2)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/ 
commctrl.c:421:close_getadapter_fib((tainted
1)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/ 
commctrl.c:348:aac_close_fib_context((tainted
1))]

	while (!list_empty(&fibctx->fibs)) {
		struct list_head * entry;
		/*
		 *	Pull the next fib from the fibs
		 */

Error --->
		entry = fibctx->fibs.next;
		list_del(entry);
		fib = list_entry(entry, struct hw_fib, header.FibLinks);
		fibctx->count--;
---------------------------------------------------------

As it turns out, the driver is fine. It is dereferencing a  
user-supplied pointer (fibctx), but it keeps a list of valid structures  
and has already made sure fibctx is one of them before using it. This  
is in contrast to the PCMCIA code, which uses a magic number to verify  
(as discussed yesterday) rather than keeping a list of all valid  
pointers.

The attached aacraid patch may help the checker, and should be  
functionally equivalent... but isn't necessary.

-- 
Hollis Blanchard
IBM Linux Technology Center

[-- Attachment #2: aacraid-userptr.diff --]
[-- Type: application/octet-stream, Size: 2080 bytes --]

===== drivers/scsi/aacraid/commctrl.c 1.2 vs edited =====
--- 1.2/drivers/scsi/aacraid/commctrl.c	Wed May  7 21:08:36 2003
+++ edited/drivers/scsi/aacraid/commctrl.c	Thu May 29 17:33:17 2003
@@ -214,8 +214,8 @@
 	if (found == 0)
 		return -EINVAL;
 
-	if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
-		 (fibctx->size != sizeof(struct aac_fib_context)))
+	if((aifcp->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+		 (aifcp->size != sizeof(struct aac_fib_context)))
 		return -EINVAL;
 	status = 0;
 	spin_lock_irqsave(&dev->fib_lock, flags);
@@ -224,16 +224,16 @@
 	 *	-EAGAIN
 	 */
 return_fib:
-	if (!aac_list_empty(&fibctx->hw_fib_list)) {
+	if (!aac_list_empty(&aifcp->hw_fib_list)) {
 		struct aac_list_head * entry;
 		/*
 		 *	Pull the next fib from the fibs
 		 */
-		entry = (struct aac_list_head*)(ulong)fibctx->hw_fib_list.next;
+		entry = (struct aac_list_head*)(ulong)aifcp->hw_fib_list.next;
 		aac_list_del(entry);
 		
 		hw_fib = aac_list_entry(entry, struct hw_fib, header.FibLinks);
-		fibctx->count--;
+		aifcp->count--;
 		spin_unlock_irqrestore(&dev->fib_lock, flags);
 		if (copy_to_user(f.fib, hw_fib, sizeof(struct hw_fib))) {
 			kfree(hw_fib);
@@ -244,11 +244,11 @@
 		 */
 		kfree(hw_fib);
 		status = 0;
-		fibctx->jiffies = jiffies/HZ;
+		aifcp->jiffies = jiffies/HZ;
 	} else {
 		spin_unlock_irqrestore(&dev->fib_lock, flags);
 		if (f.wait) {
-			if(down_interruptible(&fibctx->wait_sem) < 0) {
+			if(down_interruptible(&aifcp->wait_sem) < 0) {
 				status = -EINTR;
 			} else {
 				/* Lock again and retry */
@@ -341,11 +341,11 @@
 	if(found == 0)
 		return 0; /* Already gone */
 
-	if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
-		 (fibctx->size != sizeof(struct aac_fib_context)))
+	if((aifcp->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+		 (aifcp->size != sizeof(struct aac_fib_context)))
 		return -EINVAL;
 	spin_lock_irqsave(&dev->fib_lock, flags);
-	status = aac_close_fib_context(dev, fibctx);
+	status = aac_close_fib_context(dev, aifcp);
 	spin_unlock_irqrestore(&dev->fib_lock, flags);
 	return status;
 }