tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 633b47cb009d09dc8f4ba9cdb3a0ca138809c7c7
commit: 23066c3f4e2146da8c7d1505729f4409f4d93d28 Compiler Attributes: enable -Wstringop-truncation on W=1 (gcc >= 8)
date: 5 years ago
config: arm-randconfig-002-20230928 (https://download.01.org/0day-ci/archive/20230928/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230928/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In function 'write_hints',
inlined from 'dm_cache_write_hints' at drivers/md/dm-cache-metadata.c:1729:6:
>> drivers/md/dm-cache-metadata.c:1705:9: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
1705 | strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/strncpy +1705 drivers/md/dm-cache-metadata.c
4e781b498ee500 Joe Thornber 2016-09-15 1689
4e781b498ee500 Joe Thornber 2016-09-15 1690 /*
4e781b498ee500 Joe Thornber 2016-09-15 1691 * It's quicker to always delete the hint array, and recreate with
4e781b498ee500 Joe Thornber 2016-09-15 1692 * dm_array_new().
4e781b498ee500 Joe Thornber 2016-09-15 1693 */
4e781b498ee500 Joe Thornber 2016-09-15 1694 static int write_hints(struct dm_cache_metadata *cmd, struct dm_cache_policy *policy)
c6b4fcbad044e6 Joe Thornber 2013-03-01 1695 {
c6b4fcbad044e6 Joe Thornber 2013-03-01 1696 int r;
c6b4fcbad044e6 Joe Thornber 2013-03-01 1697 size_t hint_size;
c6b4fcbad044e6 Joe Thornber 2013-03-01 1698 const char *policy_name = dm_cache_policy_get_name(policy);
4e7f506f642963 Mike Snitzer 2013-03-20 1699 const unsigned *policy_version = dm_cache_policy_get_version(policy);
c6b4fcbad044e6 Joe Thornber 2013-03-01 1700
c6b4fcbad044e6 Joe Thornber 2013-03-01 1701 if (!policy_name[0] ||
c6b4fcbad044e6 Joe Thornber 2013-03-01 1702 (strlen(policy_name) > sizeof(cmd->policy_name) - 1))
c6b4fcbad044e6 Joe Thornber 2013-03-01 1703 return -EINVAL;
c6b4fcbad044e6 Joe Thornber 2013-03-01 1704
c6b4fcbad044e6 Joe Thornber 2013-03-01 @1705 strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
4e7f506f642963 Mike Snitzer 2013-03-20 1706 memcpy(cmd->policy_version, policy_version, sizeof(cmd->policy_version));
c6b4fcbad044e6 Joe Thornber 2013-03-01 1707
c6b4fcbad044e6 Joe Thornber 2013-03-01 1708 hint_size = dm_cache_policy_get_hint_size(policy);
c6b4fcbad044e6 Joe Thornber 2013-03-01 1709 if (!hint_size)
c6b4fcbad044e6 Joe Thornber 2013-03-01 1710 return 0; /* short-circuit hints initialization */
c6b4fcbad044e6 Joe Thornber 2013-03-01 1711 cmd->policy_hint_size = hint_size;
c6b4fcbad044e6 Joe Thornber 2013-03-01 1712
c6b4fcbad044e6 Joe Thornber 2013-03-01 1713 if (cmd->hint_root) {
c6b4fcbad044e6 Joe Thornber 2013-03-01 1714 r = dm_array_del(&cmd->hint_info, cmd->hint_root);
c6b4fcbad044e6 Joe Thornber 2013-03-01 1715 if (r)
c6b4fcbad044e6 Joe Thornber 2013-03-01 1716 return r;
c6b4fcbad044e6 Joe Thornber 2013-03-01 1717 }
c6b4fcbad044e6 Joe Thornber 2013-03-01 1718
4e781b498ee500 Joe Thornber 2016-09-15 1719 return dm_array_new(&cmd->hint_info, &cmd->hint_root,
c6b4fcbad044e6 Joe Thornber 2013-03-01 1720 from_cblock(cmd->cache_blocks),
4e781b498ee500 Joe Thornber 2016-09-15 1721 get_hint, policy);
0596661f0a16d9 Joe Thornber 2014-04-03 1722 }
0596661f0a16d9 Joe Thornber 2014-04-03 1723
:::::: The code at line 1705 was first introduced by commit
:::::: c6b4fcbad044e6fffcc75bba160e720eb8d67d17 dm: add cache target
:::::: TO: Joe Thornber <[email protected]>
:::::: CC: Alasdair G Kergon <[email protected]>
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Sep 28, 2023 at 12:24 AM kernel test robot <[email protected]> wrote:
>
> In function 'write_hints',
> inlined from 'dm_cache_write_hints' at drivers/md/dm-cache-metadata.c:1729:6:
> >> drivers/md/dm-cache-metadata.c:1705:9: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
> 1705 | strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This looks fine given the check above it. In any case, since it
appears the code wants to terminate the string, this should use
`strscpy*()`, right?
Cheers,
Miguel
On Sat, Sep 30, 2023 at 2:49 AM Kees Cook <[email protected]> wrote:
>
> On Thu, Sep 28, 2023 at 11:36:46AM +0200, Miguel Ojeda wrote:
> > On Thu, Sep 28, 2023 at 12:24 AM kernel test robot <[email protected]> wrote:
> > >
> > > In function 'write_hints',
> > > inlined from 'dm_cache_write_hints' at drivers/md/dm-cache-metadata.c:1729:6:
> > > >> drivers/md/dm-cache-metadata.c:1705:9: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
> > > 1705 | strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This looks fine given the check above it. In any case, since it
> > appears the code wants to terminate the string, this should use
> > `strscpy*()`, right?
>
> I agree. I can't tell if _pad is needed though. I think struct
> dm_cache_metadata is only used internally? And I see at least the
> initial allocation is zeroed:
>
> cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
>
> Regardless, for background,
>
> struct has:
> char policy_name[CACHE_POLICY_NAME_SIZE];
>
> code does:
> const char *policy_name = dm_cache_policy_get_name(policy);
>
> if (!policy_name[0] ||
> (strlen(policy_name) > sizeof(cmd->policy_name) - 1))
> return -EINVAL;
>
> strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
>
>
> Justin, can you add this to your queue (if it's not already there)?
I sent a patch out earlier this week [1].
The line # on the diff v.s the warning is off though. I suspect I may
need a rebase if it doesn't apply clean.
>
> -Kees
>
> --
> Kees Cook
[1]: https://lore.kernel.org/all/20230925-strncpy-drivers-md-dm-cache-metadata-c-v1-1-4b75c7db0cfe@google.com/
On Thu, Sep 28, 2023 at 11:36:46AM +0200, Miguel Ojeda wrote:
> On Thu, Sep 28, 2023 at 12:24 AM kernel test robot <[email protected]> wrote:
> >
> > In function 'write_hints',
> > inlined from 'dm_cache_write_hints' at drivers/md/dm-cache-metadata.c:1729:6:
> > >> drivers/md/dm-cache-metadata.c:1705:9: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
> > 1705 | strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This looks fine given the check above it. In any case, since it
> appears the code wants to terminate the string, this should use
> `strscpy*()`, right?
I agree. I can't tell if _pad is needed though. I think struct
dm_cache_metadata is only used internally? And I see at least the
initial allocation is zeroed:
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
Regardless, for background,
struct has:
char policy_name[CACHE_POLICY_NAME_SIZE];
code does:
const char *policy_name = dm_cache_policy_get_name(policy);
if (!policy_name[0] ||
(strlen(policy_name) > sizeof(cmd->policy_name) - 1))
return -EINVAL;
strncpy(cmd->policy_name, policy_name, sizeof(cmd->policy_name));
Justin, can you add this to your queue (if it's not already there)?
-Kees
--
Kees Cook