Message ID | 1579983942-11927-1-git-send-email-adilger@dilger.ca |
---|---|
State | Superseded |
Headers | show |
Series | ext4: don't assume that mmp_nodename/bdevname have NUL | expand |
Hi Andreas, I love your patch! Perhaps something to improve: [auto build test WARNING on ext4/dev] [also build test WARNING on tytso-fscrypt/master v5.5-rc7 next-20200124] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Andreas-Dilger/ext4-don-t-assume-that-mmp_nodename-bdevname-have-NUL/20200126-053627 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev config: x86_64-defconfig (attached as .config) compiler: gcc-7 (Debian 7.5.0-3) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): fs/ext4/mmp.c: In function '__dump_mmp_msg': >> fs/ext4/mmp.c:123:73: warning: field precision specifier '.*' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=] "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s", ~~^~ fs/ext4/mmp.c:123:99: warning: field precision specifier '.*' expects argument of type 'int', but argument 8 has type 'long unsigned int' [-Wformat=] "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s", ~~^~ In file included from fs/ext4/mmp.c:6:0: fs/ext4/mmp.c: In function 'ext4_multi_mount_protect': fs/ext4/mmp.c:382:57: warning: field precision specifier '.*' expects argument of type 'int', but argument 5 has type 'long unsigned int' [-Wformat=] EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", ^ include/linux/kthread.h:26:55: note: in definition of macro 'kthread_create' kthread_create_on_node(threadfn, data, NUMA_NO_NODE, namefmt, ##arg) ^~~~~~~ >> fs/ext4/mmp.c:382:27: note: in expansion of macro 'kthread_run' EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", ^~~~~~~~~~~ vim +123 fs/ext4/mmp.c 114 115 /* 116 * Dump as much information as possible to help the admin. 117 */ 118 void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp, 119 const char *function, unsigned int line, const char *msg) 120 { 121 __ext4_warning(sb, function, line, "%s", msg); 122 __ext4_warning(sb, function, line, > 123 "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s", 124 (long long unsigned int)le64_to_cpu(mmp->mmp_time), 125 sizeof(mmp->mmp_nodename), mmp->mmp_nodename, 126 sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname); 127 } 128 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
Hi Andreas, I love your patch! Perhaps something to improve: [auto build test WARNING on ext4/dev] [also build test WARNING on tytso-fscrypt/master v5.5-rc7 next-20200124] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Andreas-Dilger/ext4-don-t-assume-that-mmp_nodename-bdevname-have-NUL/20200126-053627 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev config: x86_64-randconfig-s0-20200126 (attached as .config) compiler: gcc-4.9 (Debian 4.9.2-10+deb8u1) 4.9.2 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): fs//ext4/mmp.c: In function '__dump_mmp_msg': fs//ext4/mmp.c:126:10: warning: field precision specifier '.*' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=] sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname); ^ fs//ext4/mmp.c:126:10: warning: field precision specifier '.*' expects argument of type 'int', but argument 8 has type 'long unsigned int' [-Wformat=] In file included from fs//ext4/mmp.c:6:0: fs//ext4/mmp.c: In function 'ext4_multi_mount_protect': >> include/linux/kthread.h:45:9: warning: field precision specifier '.*' expects argument of type 'int', but argument 5 has type 'long unsigned int' [-Wformat=] struct task_struct *__k \ ^ fs//ext4/mmp.c:382:27: note: in expansion of macro 'kthread_run' EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", ^ -- fs/ext4/mmp.c: In function '__dump_mmp_msg': fs/ext4/mmp.c:126:10: warning: field precision specifier '.*' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=] sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname); ^ fs/ext4/mmp.c:126:10: warning: field precision specifier '.*' expects argument of type 'int', but argument 8 has type 'long unsigned int' [-Wformat=] In file included from fs/ext4/mmp.c:6:0: fs/ext4/mmp.c: In function 'ext4_multi_mount_protect': >> include/linux/kthread.h:45:9: warning: field precision specifier '.*' expects argument of type 'int', but argument 5 has type 'long unsigned int' [-Wformat=] struct task_struct *__k \ ^ fs/ext4/mmp.c:382:27: note: in expansion of macro 'kthread_run' EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", ^ vim +45 include/linux/kthread.h ^1da177e4c3f41 Linus Torvalds 2005-04-16 7 b9075fa968a0a4 Joe Perches 2011-10-31 8 __printf(4, 5) 207205a2ba2655 Eric Dumazet 2011-03-22 9 struct task_struct *kthread_create_on_node(int (*threadfn)(void *data), ^1da177e4c3f41 Linus Torvalds 2005-04-16 10 void *data, 207205a2ba2655 Eric Dumazet 2011-03-22 11 int node, b9075fa968a0a4 Joe Perches 2011-10-31 12 const char namefmt[], ...); 207205a2ba2655 Eric Dumazet 2011-03-22 13 e154ccc831b5b5 Jonathan Corbet 2016-10-11 14 /** e154ccc831b5b5 Jonathan Corbet 2016-10-11 15 * kthread_create - create a kthread on the current node e154ccc831b5b5 Jonathan Corbet 2016-10-11 16 * @threadfn: the function to run in the thread e154ccc831b5b5 Jonathan Corbet 2016-10-11 17 * @data: data pointer for @threadfn() e154ccc831b5b5 Jonathan Corbet 2016-10-11 18 * @namefmt: printf-style format string for the thread name d16977f3a6cfbb Jonathan Corbet 2017-08-02 19 * @arg...: arguments for @namefmt. e154ccc831b5b5 Jonathan Corbet 2016-10-11 20 * e154ccc831b5b5 Jonathan Corbet 2016-10-11 21 * This macro will create a kthread on the current node, leaving it in e154ccc831b5b5 Jonathan Corbet 2016-10-11 22 * the stopped state. This is just a helper for kthread_create_on_node(); e154ccc831b5b5 Jonathan Corbet 2016-10-11 23 * see the documentation there for more details. e154ccc831b5b5 Jonathan Corbet 2016-10-11 24 */ 207205a2ba2655 Eric Dumazet 2011-03-22 25 #define kthread_create(threadfn, data, namefmt, arg...) \ e9f069868d6055 Andrew Morton 2015-09-04 26 kthread_create_on_node(threadfn, data, NUMA_NO_NODE, namefmt, ##arg) 207205a2ba2655 Eric Dumazet 2011-03-22 27 ^1da177e4c3f41 Linus Torvalds 2005-04-16 28 2a1d446019f9a5 Thomas Gleixner 2012-07-16 29 struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), 2a1d446019f9a5 Thomas Gleixner 2012-07-16 30 void *data, 2a1d446019f9a5 Thomas Gleixner 2012-07-16 31 unsigned int cpu, 2a1d446019f9a5 Thomas Gleixner 2012-07-16 32 const char *namefmt); 2a1d446019f9a5 Thomas Gleixner 2012-07-16 33 ^1da177e4c3f41 Linus Torvalds 2005-04-16 34 /** 9e37bd301ee130 Randy Dunlap 2006-06-25 35 * kthread_run - create and wake a thread. ^1da177e4c3f41 Linus Torvalds 2005-04-16 36 * @threadfn: the function to run until signal_pending(current). ^1da177e4c3f41 Linus Torvalds 2005-04-16 37 * @data: data ptr for @threadfn. ^1da177e4c3f41 Linus Torvalds 2005-04-16 38 * @namefmt: printf-style name for the thread. ^1da177e4c3f41 Linus Torvalds 2005-04-16 39 * ^1da177e4c3f41 Linus Torvalds 2005-04-16 40 * Description: Convenient wrapper for kthread_create() followed by 9e37bd301ee130 Randy Dunlap 2006-06-25 41 * wake_up_process(). Returns the kthread or ERR_PTR(-ENOMEM). 9e37bd301ee130 Randy Dunlap 2006-06-25 42 */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 43 #define kthread_run(threadfn, data, namefmt, ...) \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 44 ({ \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 @45 struct task_struct *__k \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 46 = kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 47 if (!IS_ERR(__k)) \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 48 wake_up_process(__k); \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 49 __k; \ ^1da177e4c3f41 Linus Torvalds 2005-04-16 50 }) ^1da177e4c3f41 Linus Torvalds 2005-04-16 51 :::::: The code at line 45 was first introduced by commit :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2 :::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org> :::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
On Jan 26, 2020, at 3:03 PM, Andreas Dilger <adilger@dilger.ca> wrote: > > Don't assume that the mmp_nodename and mmp_bdevname strings are NUL > terminated, since they are filled in by snprintf(), which is not > guaranteed to do so. > > Signed-off-by: Andreas Dilger <adilger@dilger.ca> > --- NB: this is v2 of the patch, which fixes the checkpatch warnings. Ted, do you also want an ext4 patch with EXT4_LEN_STR() and a change of these char strings to __u8, along with similar changes to other non-NUL-terminated strings in the superblock, as was done for e2fsprogs? Cheers, Andreas > fs/ext4/mmp.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c > index 2305b43..9d00e0d 100644 > --- a/fs/ext4/mmp.c > +++ b/fs/ext4/mmp.c > @@ -120,10 +120,10 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp, > { > __ext4_warning(sb, function, line, "%s", msg); > __ext4_warning(sb, function, line, > - "MMP failure info: last update time: %llu, last update " > - "node: %s, last update device: %s", > - (long long unsigned int) le64_to_cpu(mmp->mmp_time), > - mmp->mmp_nodename, mmp->mmp_bdevname); > + "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s", > + (unsigned long long)le64_to_cpu(mmp->mmp_time), > + (int)sizeof(mmp->mmp_nodename), mmp->mmp_nodename, > + (int)sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname); > } > > /* > @@ -154,6 +154,7 @@ static int kmmpd(void *data) > mmp_check_interval = max(EXT4_MMP_CHECK_MULT * mmp_update_interval, > EXT4_MMP_MIN_CHECK_INTERVAL); > mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval); > + BUILD_BUG_ON(sizeof(mmp->mmp_bdevname) < BDEVNAME_SIZE); > bdevname(bh->b_bdev, mmp->mmp_bdevname); > > memcpy(mmp->mmp_nodename, init_utsname()->nodename, > @@ -375,7 +376,8 @@ int ext4_multi_mount_protect(struct super_block *sb, > /* > * Start a kernel thread to update the MMP block periodically. > */ > - EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%s", > + EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", > + (int)sizeof(mmp->mmp_bdevname), > bdevname(bh->b_bdev, > mmp->mmp_bdevname)); > if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) { > -- > 1.8.0 > Cheers, Andreas
On Sun, Jan 26, 2020 at 03:06:36PM -0700, Andreas Dilger wrote: > Ted, do you also want an ext4 patch with EXT4_LEN_STR() and a change of these > char strings to __u8, along with similar changes to other non-NUL-terminated > strings in the superblock, as was done for e2fsprogs? Yes, it would be great to have that as a separate cleanup patch, thanks. - Ted
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c index 2305b43..612476e 100644 --- a/fs/ext4/mmp.c +++ b/fs/ext4/mmp.c @@ -120,10 +120,10 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp, { __ext4_warning(sb, function, line, "%s", msg); __ext4_warning(sb, function, line, - "MMP failure info: last update time: %llu, last update " - "node: %s, last update device: %s", - (long long unsigned int) le64_to_cpu(mmp->mmp_time), - mmp->mmp_nodename, mmp->mmp_bdevname); + "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s", + (long long unsigned int)le64_to_cpu(mmp->mmp_time), + sizeof(mmp->mmp_nodename), mmp->mmp_nodename, + sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname); } /* @@ -375,7 +375,8 @@ int ext4_multi_mount_protect(struct super_block *sb, /* * Start a kernel thread to update the MMP block periodically. */ - EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%s", + EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s", + sizeof(mmp->mmp_bdevname), bdevname(bh->b_bdev, mmp->mmp_bdevname)); if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {
Don't assume that the mmp_nodename and mmp_bdevname strings are NUL terminated, since they are filled in by snprintf(), which is not guaranteed to do so. Signed-off-by: Andreas Dilger <adilger@dilger.ca> --- fs/ext4/mmp.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)