linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mathieu Malaterre <malat@debian.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Malaterre <malat@debian.org>,
	Nick Desaulniers <nick.desaulniers@gmail.com>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] writeback: tracing: Copy only up to 31 characters in strncpy() calls
Date: Fri, 18 Jan 2019 20:32:00 +0100	[thread overview]
Message-ID: <20190118193200.32691-1-malat@debian.org> (raw)

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.desaulniers@gmail.com>
Link: https://lore.kernel.org/patchwork/patch/910830/
Signed-off-by: Mathieu Malaterre <malat@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);
 		__entry->ino = mapping ? mapping->host->i_ino : 0;
 		__entry->index = page->index;
 	),
@@ -97,7 +98,8 @@ DECLARE_EVENT_CLASS(writeback_dirty_inode_template,
 
 		/* may be called for files on pseudo FSes w/ unregistered bdi */
 		strncpy(__entry->name,
-			bdi->dev ? dev_name(bdi->dev) : "(unknown)", 32);
+			bdi->dev ? dev_name(bdi->dev) : "(unknown)",
+			sizeof(__entry->name) - 1);
 		__entry->ino		= inode->i_ino;
 		__entry->state		= inode->i_state;
 		__entry->flags		= flags;
@@ -177,7 +179,8 @@ DECLARE_EVENT_CLASS(writeback_write_inode_template,
 
 	TP_fast_assign(
 		strncpy(__entry->name,
-			dev_name(inode_to_bdi(inode)->dev), 32);
+			dev_name(inode_to_bdi(inode)->dev),
+			sizeof(__entry->name) - 1);
 		__entry->ino		= inode->i_ino;
 		__entry->sync_mode	= wbc->sync_mode;
 		__entry->cgroup_ino	= __trace_wbc_assign_cgroup(wbc);
@@ -221,7 +224,8 @@ DECLARE_EVENT_CLASS(writeback_work_class,
 	),
 	TP_fast_assign(
 		strncpy(__entry->name,
-			wb->bdi->dev ? dev_name(wb->bdi->dev) : "(unknown)", 32);
+			wb->bdi->dev ? dev_name(wb->bdi->dev) : "(unknown)",
+			sizeof(__entry->name) - 1);
 		__entry->nr_pages = work->nr_pages;
 		__entry->sb_dev = work->sb ? work->sb->s_dev : 0;
 		__entry->sync_mode = work->sync_mode;
@@ -274,7 +278,8 @@ DECLARE_EVENT_CLASS(writeback_class,
 		__field(unsigned int, cgroup_ino)
 	),
 	TP_fast_assign(
-		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
+		strncpy(__entry->name, dev_name(wb->bdi->dev),
+			sizeof(__entry->name) - 1);
 		__entry->cgroup_ino = __trace_wb_assign_cgroup(wb);
 	),
 	TP_printk("bdi %s: cgroup_ino=%u",
@@ -296,7 +301,8 @@ TRACE_EVENT(writeback_bdi_register,
 		__array(char, name, 32)
 	),
 	TP_fast_assign(
-		strncpy(__entry->name, dev_name(bdi->dev), 32);
+		strncpy(__entry->name, dev_name(bdi->dev),
+			sizeof(__entry->name) - 1);
 	),
 	TP_printk("bdi %s",
 		__entry->name
@@ -321,7 +327,8 @@ DECLARE_EVENT_CLASS(wbc_class,
 	),
 
 	TP_fast_assign(
-		strncpy(__entry->name, dev_name(bdi->dev), 32);
+		strncpy(__entry->name, dev_name(bdi->dev),
+			sizeof(__entry->name) - 1);
 		__entry->nr_to_write	= wbc->nr_to_write;
 		__entry->pages_skipped	= wbc->pages_skipped;
 		__entry->sync_mode	= wbc->sync_mode;
@@ -372,7 +379,8 @@ TRACE_EVENT(writeback_queue_io,
 	),
 	TP_fast_assign(
 		unsigned long *older_than_this = work->older_than_this;
-		strncpy(__entry->name, dev_name(wb->bdi->dev), 32);
+		strncpy(__entry->name, dev_name(wb->bdi->dev),
+			sizeof(__entry->name) - 1);
 		__entry->older	= older_than_this ?  *older_than_this : 0;
 		__entry->age	= older_than_this ?
 				  (jiffies - *older_than_this) * 1000 / HZ : -1;
@@ -584,7 +592,8 @@ TRACE_EVENT(writeback_sb_inodes_requeue,
 
 	TP_fast_assign(
 		strncpy(__entry->name,
-		        dev_name(inode_to_bdi(inode)->dev), 32);
+			dev_name(inode_to_bdi(inode)->dev),
+			sizeof(__entry->name) - 1);
 		__entry->ino		= inode->i_ino;
 		__entry->state		= inode->i_state;
 		__entry->dirtied_when	= inode->dirtied_when;
@@ -658,7 +667,8 @@ DECLARE_EVENT_CLASS(writeback_single_inode_template,
 
 	TP_fast_assign(
 		strncpy(__entry->name,
-			dev_name(inode_to_bdi(inode)->dev), 32);
+			dev_name(inode_to_bdi(inode)->dev),
+			sizeof(__entry->name) - 1);
 		__entry->ino		= inode->i_ino;
 		__entry->state		= inode->i_state;
 		__entry->dirtied_when	= inode->dirtied_when;
-- 
2.19.2


             reply	other threads:[~2019-01-18 19:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-18 19:32 Mathieu Malaterre [this message]
2019-01-25  4:26 ` [PATCH] writeback: tracing: Copy only up to 31 characters in strncpy() calls Nick Desaulniers
2019-01-25  7:58   ` Mathieu Malaterre
2019-01-25 17:06     ` Kees Cook
2019-01-31 10:58       ` Mathieu Malaterre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190118193200.32691-1-malat@debian.org \
    --to=malat@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nick.desaulniers@gmail.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).