* [patch] ntb_perf: potential info leak in debugfs
@ 2016-10-14 7:34 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-10-14 7:34 UTC (permalink / raw)
To: Jon Mason, Logan Gunthorpe
Cc: Dave Jiang, Allen Hubbe, Sudip Mukherjee, Arnd Bergmann,
linux-ntb, kernel-janitors
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 <dan.carpenter@oracle.com>
diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 6a50f20..2d9ca58 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -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);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [patch] ntb_perf: potential info leak in debugfs
2016-10-14 7:34 ` Dan Carpenter
@ 2016-10-14 17:07 ` Dave Jiang
-1 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2016-10-14 17:07 UTC (permalink / raw)
To: kernel-janitors
On 10/14/2016 12:34 AM, Dan Carpenter wrote:
> 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 <dan.carpenter@oracle.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>
>
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 6a50f20..2d9ca58 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -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);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] ntb_perf: potential info leak in debugfs
@ 2016-10-14 17:07 ` Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2016-10-14 17:07 UTC (permalink / raw)
To: Dan Carpenter, Jon Mason, Logan Gunthorpe
Cc: Allen Hubbe, Sudip Mukherjee, Arnd Bergmann, linux-ntb,
kernel-janitors
On 10/14/2016 12:34 AM, Dan Carpenter wrote:
> 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 <dan.carpenter@oracle.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>
>
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 6a50f20..2d9ca58 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -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);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ntb_perf: potential info leak in debugfs
2016-10-14 17:07 ` Dave Jiang
@ 2016-11-01 20:35 ` Jon Mason
-1 siblings, 0 replies; 6+ messages in thread
From: Jon Mason @ 2016-11-01 20:35 UTC (permalink / raw)
To: kernel-janitors
On Fri, Oct 14, 2016 at 10:07:13AM -0700, Dave Jiang wrote:
>
>
> On 10/14/2016 12:34 AM, Dan Carpenter wrote:
> > 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 <dan.carpenter@oracle.com>
> Acked-by: Dave Jiang <dave.jiang@intel.com>
Sorry for the delay. Pulled into my ntb branch.
Thanks,
Jon
>
> >
> > diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> > index 6a50f20..2d9ca58 100644
> > --- a/drivers/ntb/test/ntb_perf.c
> > +++ b/drivers/ntb/test/ntb_perf.c
> > @@ -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);
> > }
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] ntb_perf: potential info leak in debugfs
@ 2016-11-01 20:35 ` Jon Mason
0 siblings, 0 replies; 6+ messages in thread
From: Jon Mason @ 2016-11-01 20:35 UTC (permalink / raw)
To: Dave Jiang
Cc: Dan Carpenter, Logan Gunthorpe, Allen Hubbe, Sudip Mukherjee,
Arnd Bergmann, linux-ntb, kernel-janitors
On Fri, Oct 14, 2016 at 10:07:13AM -0700, Dave Jiang wrote:
>
>
> On 10/14/2016 12:34 AM, Dan Carpenter wrote:
> > 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 <dan.carpenter@oracle.com>
> Acked-by: Dave Jiang <dave.jiang@intel.com>
Sorry for the delay. Pulled into my ntb branch.
Thanks,
Jon
>
> >
> > diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> > index 6a50f20..2d9ca58 100644
> > --- a/drivers/ntb/test/ntb_perf.c
> > +++ b/drivers/ntb/test/ntb_perf.c
> > @@ -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);
> > }
> >
^ permalink raw reply [flat|nested] 6+ messages in thread