Skip to contentSkip to conversation

ntb_perf: potential info leak in debugfs

PATCH In review Dan Carpenter 3 participants 3 messages Oct 14, 2016 – Nov 1, 2016

Subsystems (2)
Kernel janitors NTB driver core
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 ntb_perf: potential info leak in debugfs

This is a static checker warning, not something I'm desperately concerned about. But snprintf() returns the number of bytes that would have been copied if there were space. We really care about the number of bytes that actually were copied so we should use scnprintf() instead.

It probably won't overrun, and in that case we may as well just use sprintf() but these sorts of things make static checkers and code reviewers happier.

Signed-off-by Dan Carpenter <[email protected]>
drivers/ntb/test/ntb_perf.c +3 -3
index 6a50f20..2d9ca58 100644
@@ -589,7 +589,7 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 		return -ENOMEM;
 
 	if (mutex_is_locked(&perf->run_mutex)) {
-		out_off = snprintf(buf, 64, "running\n");
+		out_off = scnprintf(buf, 64, "running\n");
 		goto read_from_buf;
 	}
 

@@ -600,14 +600,14 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 			break;
 
 		if (pctx->status) {
-			out_off += snprintf(buf + out_off, 1024 - out_off,
+			out_off += scnprintf(buf + out_off, 1024 - out_off,
 					    "%d: error %d\n", i,
 					    pctx->status);
 			continue;
 		}
 
 		rate = div64_u64(pctx->copied, pctx->diff_us);
-		out_off += snprintf(buf + out_off, 1024 - out_off,
+		out_off += scnprintf(buf + out_off, 1024 - out_off,
 			"%d: copied %llu bytes in %llu usecs, %llu MBytes/s\n",
 			i, pctx->copied, pctx->diff_us, rate);
 	}
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 11 lines · Dan Carpenter

This is a static checker warning, not something I'm desperately concerned about. But snprintf() returns the number of bytes that would have been copied if there were space. We really care about the number of bytes that actually were copied so we should use scnprintf() instead.

It probably won't overrun, and in that case we may as well just use sprintf() but these sorts of things make static checkers and code reviewers happier.

Signed-off-by Dan Carpenter <[email protected]>
Acked-by Dave Jiang <[email protected]>
Quoted 32 lines
drivers/ntb/test/ntb_perf.c +3 -3
index 6a50f20..2d9ca58 100644
@@ -589,7 +589,7 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 		return -ENOMEM;
 
 	if (mutex_is_locked(&perf->run_mutex)) {
-		out_off = snprintf(buf, 64, "running\n");
+		out_off = scnprintf(buf, 64, "running\n");
 		goto read_from_buf;
 	}
 

@@ -600,14 +600,14 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 			break;
 
 		if (pctx->status) {
-			out_off += snprintf(buf + out_off, 1024 - out_off,
+			out_off += scnprintf(buf + out_off, 1024 - out_off,
 					    "%d: error %d\n", i,
 					    pctx->status);
 			continue;
 		}
 
 		rate = div64_u64(pctx->copied, pctx->diff_us);
-		out_off += snprintf(buf + out_off, 1024 - out_off,
+		out_off += scnprintf(buf + out_off, 1024 - out_off,
 			"%d: copied %llu bytes in %llu usecs, %llu MBytes/s\n",
 			i, pctx->copied, pctx->diff_us, rate);
 	}
maintainer
#2 lore raw
In reply to Dan Carpenter
Quoted 9 lines · Dave Jiang
Quoted 6 lines · Dan Carpenter

This is a static checker warning, not something I'm desperately concerned about. But snprintf() returns the number of bytes that would have been copied if there were space. We really care about the number of bytes that actually were copied so we should use scnprintf() instead.

Quoted 6 lines
Quoted 5 lines

It probably won't overrun, and in that case we may as well just use sprintf() but these sorts of things make static checkers and code reviewers happier.

Signed-off-by Dan Carpenter <[email protected]>
Acked-by Dave Jiang <[email protected]>

Sorry for the delay. Pulled into my ntb branch.

Thanks,
Jon

Quoted 33 lines
Quoted 33 lines
drivers/ntb/test/ntb_perf.c +3 -3
index 6a50f20..2d9ca58 100644
@@ -589,7 +589,7 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 		return -ENOMEM;
 
 	if (mutex_is_locked(&perf->run_mutex)) {
-		out_off = snprintf(buf, 64, "running\n");
+		out_off = scnprintf(buf, 64, "running\n");
 		goto read_from_buf;
 	}
 

@@ -600,14 +600,14 @@ static ssize_t debugfs_run_read(struct file *filp, char __user *ubuf,
 			break;
 
 		if (pctx->status) {
-			out_off += snprintf(buf + out_off, 1024 - out_off,
+			out_off += scnprintf(buf + out_off, 1024 - out_off,
 					    "%d: error %d\n", i,
 					    pctx->status);
 			continue;
 		}
 
 		rate = div64_u64(pctx->copied, pctx->diff_us);
-		out_off += snprintf(buf + out_off, 1024 - out_off,
+		out_off += scnprintf(buf + out_off, 1024 - out_off,
 			"%d: copied %llu bytes in %llu usecs, %llu MBytes/s\n",
 			i, pctx->copied, pctx->diff_us, rate);
 	}