Skip to contentSkip to conversation

char: ip2: check put_user() result

PATCH In review Kulikov Vasiliy 2 participants 2 messages Jul 29, 2010

Subsystems (2)
Kernel janitors Char and misc drivers
lore
Share & tools
lore

Kulikov Vasiliy

author
#0 lore raw

PATCH 1/1 char: ip2: check put_user() result

put_user()/copy_to_user() may fail, if so return -EFAULT.

Signed-off-by Kulikov Vasiliy <[email protected]>
---
 drivers/char/ip2/ip2main.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
drivers/char/ip2/ip2main.c +8 -8
index 07f3ea3..3f70a05 100644
@@ -2813,9 +2813,9 @@ DumpTraceBuffer ( char __user *pData, int count )
 		return -EIO;
 	}
 	rc = put_user(tracewrap, pIndex );
-	rc = put_user(TRACEMAX, ++pIndex );
-	rc = put_user(tracestrip, ++pIndex );
-	rc = put_user(tracestuff, ++pIndex );
+	rc |= put_user(TRACEMAX, ++pIndex);
+	rc |= put_user(tracestrip, ++pIndex);
+	rc |= put_user(tracestuff, ++pIndex);
 	pData += sizeof(int) * 6;
 	count -= sizeof(int) * 6;
 

@@ -2828,7 +2828,7 @@ DumpTraceBuffer ( char __user *pData, int count )
 	}
 	chunk = TRACEMAX - tracestrip;
 	if ( dumpcount > chunk ) {
-		rc = copy_to_user(pData, &tracebuf[tracestrip],
+		rc |= copy_to_user(pData, &tracebuf[tracestrip],
 			      chunk * sizeof(tracebuf[0]) );
 		pData += chunk * sizeof(tracebuf[0]);
 		tracestrip = 0;

@@ -2836,15 +2836,15 @@ DumpTraceBuffer ( char __user *pData, int count )
 	} else {
 		chunk = dumpcount;
 	}
-	rc = copy_to_user(pData, &tracebuf[tracestrip],
+	rc |= copy_to_user(pData, &tracebuf[tracestrip],
 		      chunk * sizeof(tracebuf[0]) );
 	tracestrip += chunk;
 	tracewrap = 0;
 
-	rc = put_user(tracestrip, ++pIndex );
-	rc = put_user(tracestuff, ++pIndex );
+	rc |= put_user(tracestrip, ++pIndex);
+	rc |= put_user(tracestuff, ++pIndex);
 
-	return dumpcount;
+	return rc ? -EFAULT : dumpcount;
 #else
 	return 0;
 #endif
Signature · 1 line
1.7.0.4
#1 lore raw

O> - return dumpcount;

drivers/char/ip2/ip2main.c DumpTraceBuffer ( char __user *pData, int count ) · line 2847

Quoted 1 line
drivers/char/ip2/ip2main.c +1 -0
+	return rc ? -EFAULT : dumpcount;

This is actually at least as wrong as before.

The standards say that if you successfully return some data you should report the bytes returned. Normally it doesn't matter much because the data has not been 'lost' when it gets called again.

If you actually remove the data from the queue it ought to be getting reported with a suitable length.

Alan