| Message ID | 1531445162-19771-1-git-send-email-asmadeus@codewreck.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jens, I noticed this old patch in my inbox. It looks like a legit cleanup. Want to take it? -- Steve On Fri, 13 Jul 2018 03:26:02 +0200 Dominique Martinet <asmadeus@codewreck.org> wrote: > Using strlcpy fixes this new gcc warning: > kernel/trace/blktrace.c: In function ‘do_blk_trace_setup’: > kernel/trace/blktrace.c:497:2: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] > strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- > > Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the > first patch of the serie) for the motivation behind this patch > > kernel/trace/blktrace.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index 987d9a9ae283..2478d9838eab 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -494,8 +494,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, > if (!buts->buf_size || !buts->buf_nr) > return -EINVAL; > > - strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); > - buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0'; > + strlcpy(buts->name, name, BLKTRACE_BDEV_SIZE); > > /* > * some device names have larger paths - convert the slashes
On 3/14/19 7:37 PM, Steven Rostedt wrote: > > Jens, > > I noticed this old patch in my inbox. It looks like a legit cleanup. > Want to take it? Indeed, I've applied it. Thanks!
Jens, Steven, Jens Axboe wrote on Thu, Mar 14, 2019: > On 3/14/19 7:37 PM, Steven Rostedt wrote: > > I noticed this old patch in my inbox. It looks like a legit cleanup. > > Want to take it? > > Indeed, I've applied it. Thanks! Thanks. I am terribly sorry about this patch series to be honest, I did not prepare it properly and sent too many generic patches at once but more importantly some were unsafe (strlcpy expects the input string to be validly formatted, because it basically does strlen() on it to check how much hasn't been copied for its return value) I was pointed out strscpy instead as a safer alternative. In this case `name` comes from bdevname() which is disk_name() in block/partition-generic.c which is a snprintf, so we are guaranted null truncation from there and it should be OK, but I wanted to check and point it out. Anyway, thanks!
On 3/15/19 12:30 AM, Dominique Martinet wrote: > Jens, Steven, > > Jens Axboe wrote on Thu, Mar 14, 2019: >> On 3/14/19 7:37 PM, Steven Rostedt wrote: >>> I noticed this old patch in my inbox. It looks like a legit cleanup. >>> Want to take it? >> >> Indeed, I've applied it. Thanks! > > Thanks. I am terribly sorry about this patch series to be honest, I did > not prepare it properly and sent too many generic patches at once but > more importantly some were unsafe (strlcpy expects the input string to > be validly formatted, because it basically does strlen() on it to check > how much hasn't been copied for its return value) > I was pointed out strscpy instead as a safer alternative. > > In this case `name` comes from bdevname() which is disk_name() in > block/partition-generic.c which is a snprintf, so we are guaranted > null truncation from there and it should be OK, but I wanted to check > and point it out. Dropped.
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 987d9a9ae283..2478d9838eab 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -494,8 +494,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, if (!buts->buf_size || !buts->buf_nr) return -EINVAL; - strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); - buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0'; + strlcpy(buts->name, name, BLKTRACE_BDEV_SIZE); /* * some device names have larger paths - convert the slashes
Using strlcpy fixes this new gcc warning: kernel/trace/blktrace.c: In function ‘do_blk_trace_setup’: kernel/trace/blktrace.c:497:2: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation] strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Generated by scripts/coccinelle/misc/strncpy_truncation.cocci Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> --- Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the first patch of the serie) for the motivation behind this patch kernel/trace/blktrace.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)