On Fri, Jan 25, 2019 at 9:18 PM Mathieu Malaterre <ma...@debian.org> wrote: > > On Fri, Jan 25, 2019 at 5:26 AM Nick Desaulniers > <nick.desaulni...@gmail.com> wrote: > > > > On Fri, Jan 18, 2019 at 11:32 AM Mathieu Malaterre <ma...@debian.org> wrote: > > > > > > In the past an attempt was made to remove a set of warnings triggered by > > > gcc 8.x and W=1 by changing calls to strncpy() into strlcpy(). This was > > > rejected as one of the desired behavior is to keep initializing the rest > > > of the destination string whenever the source string is less than the > > > size. > > > > > > However the code makes it clear that this is not a desired behavior to > > > copy the entire 32 characters into the destination buffer, since it is > > > expected that the string is NUL terminated. So change the maximum number > > > of characters to be copied to be the size of the destination buffer > > > minus 1. > > > > > > Break long lines and make this patch go through `checkpatch --strict` with > > > no errors. > > > > > > This commit removes the following warnings: > > > > > > include/trace/events/writeback.h:69:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:99:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:179:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:223:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:277:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:299:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:324:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:375:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:586:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > include/trace/events/writeback.h:660:3: warning: 'strncpy' specified > > > bound 32 equals destination size [-Wstringop-truncation] > > > > > > Cc: Nick Desaulniers <nick.desaulni...@gmail.com> > > > Link: https://lore.kernel.org/patchwork/patch/910830/ > > > Signed-off-by: Mathieu Malaterre <ma...@debian.org> > > > --- > > > include/trace/events/writeback.h | 30 ++++++++++++++++++++---------- > > > 1 file changed, 20 insertions(+), 10 deletions(-) > > > > > > diff --git a/include/trace/events/writeback.h > > > b/include/trace/events/writeback.h > > > index 32db72c7c055..7bc58980f84f 100644 > > > --- a/include/trace/events/writeback.h > > > +++ b/include/trace/events/writeback.h > > > @@ -67,7 +67,8 @@ TRACE_EVENT(writeback_dirty_page, > > > > > > TP_fast_assign( > > > strncpy(__entry->name, > > > - mapping ? > > > dev_name(inode_to_bdi(mapping->host)->dev) : "(unknown)", 32); > > > + mapping ? > > > dev_name(inode_to_bdi(mapping->host)->dev) : > > > + "(unknown)", sizeof(__entry->name) - 1); > > > > Does strncpy guarantee that destination will be NULL terminated if > > (sizeof(src) > sizeof(dst)) || (32 > sizeof(dst))? > > No, that's the point here: > > https://www.kernel.org/doc/htmldocs/kernel-api/API-strncpy.html > > ... > The result is not NUL-terminated if the source exceeds count bytes. > > In the case where the length of src is less than that of count, the > remainder of dest will be padded with NUL. > ... > > One should use strlcpy for the use case you describe: > > https://www.kernel.org/doc/htmldocs/kernel-api/API-strlcpy.html
Please use strspy() -- strlcpy() will read the source beyond the length limit. https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.html -- Kees Cook
