| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAH7mPvi2TN0b=j-6fqDpjGPkZVSEQnDiT2nj9NWmoBOhUiwKEg@mail.gmail.com>
Date: Thu, 24 Jan 2019 20:26:24 -0800
From: Nick Desaulniers <nick.desaulniers@...il.com>
To: Mathieu Malaterre <malat@...ian.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] writeback: tracing: Copy only up to 31 characters in
strncpy() calls
On Fri, Jan 18, 2019 at 11:32 AM Mathieu Malaterre <malat@...ian.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.desaulniers@...il.com>
> Link: https://lore.kernel.org/patchwork/patch/910830/
> Signed-off-by: Mathieu Malaterre <malat@...ian.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))?
> __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
>
Powered by blists - more mailing lists