Trond/Anna, I'd like your opinion here. Some static checking flags a "ctx" assignment in nfs_fill_super() in the new patch. In an existing code there is a check for it is NULL before dereferencing. However, "ctx" can never be null. nfs_get_tree_common() which calls nfs_fill_super() and passes in "ctx" gets it from the passed in "fs_context". If the passed in arg can be null then we are dereferencing in var assignment so things would blow up there. So "ctx" can never be null. Should I create another clean up patch to remove the check for null ctx in nfs_fill_super() to make static analyzers happy? On Fri, Feb 19, 2021 at 3:19 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > Hi Olga, > > url: https://github.com/0day-ci/linux/commits/Olga-Kornievskaia/Add-new-hook-to-compare-new-mount-to-an-existing-mount/20210219-035957 > base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next > config: i386-randconfig-m021-20210215 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > smatch warnings: > fs/nfs/super.c:1061 nfs_fill_super() error: we previously assumed 'ctx' could be null (see line 1029) > > vim +/ctx +1061 fs/nfs/super.c > > 62a55d088cd87d Scott Mayhew 2019-12-10 1021 static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx) > f7b422b17ee5ee David Howells 2006-06-09 1022 { > 54ceac45159860 David Howells 2006-08-22 1023 struct nfs_server *server = NFS_SB(sb); > f7b422b17ee5ee David Howells 2006-06-09 1024 > f7b422b17ee5ee David Howells 2006-06-09 1025 sb->s_blocksize_bits = 0; > f7b422b17ee5ee David Howells 2006-06-09 1026 sb->s_blocksize = 0; > 6a74490dca8974 Bryan Schumaker 2012-07-30 1027 sb->s_xattr = server->nfs_client->cl_nfs_mod->xattr; > 6a74490dca8974 Bryan Schumaker 2012-07-30 1028 sb->s_op = server->nfs_client->cl_nfs_mod->sops; > 5eb005caf5383d David Howells 2019-12-10 @1029 if (ctx && ctx->bsize) > ^^^ > Check for NULL > > 5eb005caf5383d David Howells 2019-12-10 1030 sb->s_blocksize = nfs_block_size(ctx->bsize, &sb->s_blocksize_bits); > f7b422b17ee5ee David Howells 2006-06-09 1031 > 6a74490dca8974 Bryan Schumaker 2012-07-30 1032 if (server->nfs_client->rpc_ops->version != 2) { > 54ceac45159860 David Howells 2006-08-22 1033 /* The VFS shouldn't apply the umask to mode bits. We will do > 54ceac45159860 David Howells 2006-08-22 1034 * so ourselves when necessary. > 54ceac45159860 David Howells 2006-08-22 1035 */ > 1751e8a6cb935e Linus Torvalds 2017-11-27 1036 sb->s_flags |= SB_POSIXACL; > 54ceac45159860 David Howells 2006-08-22 1037 sb->s_time_gran = 1; > 20fa1902728698 Peng Tao 2017-06-29 1038 sb->s_export_op = &nfs_export_ops; > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1039 } else > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1040 sb->s_time_gran = 1000; > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1041 > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1042 if (server->nfs_client->rpc_ops->version != 4) { > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1043 sb->s_time_min = 0; > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1044 sb->s_time_max = U32_MAX; > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1045 } else { > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1046 sb->s_time_min = S64_MIN; > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1047 sb->s_time_max = S64_MAX; > 54ceac45159860 David Howells 2006-08-22 1048 } > f7b422b17ee5ee David Howells 2006-06-09 1049 > ab88dca311a372 Al Viro 2019-12-10 1050 sb->s_magic = NFS_SUPER_MAGIC; > 54ceac45159860 David Howells 2006-08-22 1051 > ab88dca311a372 Al Viro 2019-12-10 1052 /* We probably want something more informative here */ > ab88dca311a372 Al Viro 2019-12-10 1053 snprintf(sb->s_id, sizeof(sb->s_id), > ab88dca311a372 Al Viro 2019-12-10 1054 "%u:%u", MAJOR(sb->s_dev), MINOR(sb->s_dev)); > 1fcb79c1b21801 Deepa Dinamani 2019-03-26 1055 > ab88dca311a372 Al Viro 2019-12-10 1056 if (sb->s_blocksize == 0) > ab88dca311a372 Al Viro 2019-12-10 1057 sb->s_blocksize = nfs_block_bits(server->wsize, > ab88dca311a372 Al Viro 2019-12-10 1058 &sb->s_blocksize_bits); > f7b422b17ee5ee David Howells 2006-06-09 1059 > ab88dca311a372 Al Viro 2019-12-10 1060 nfs_super_set_maxbytes(sb, server->maxfilesize); > 52a2a3a4af9af7 Olga Kornievskaia 2021-02-18 @1061 server->has_sec_mnt_opts = ctx->has_sec_mnt_opts; > ^^^^^^^^^^^^^^^^^^^^^ > Unchecked dereference. Is the earlier NULL check necessary? (Actually > on my system with a built cross function DB, I see that the earlier > NULL check can be removed. If the cross function DB were built then > Smatch would not have printed this warning about inconsistent NULL > checks). > > f7b422b17ee5ee David Howells 2006-06-09 1062 } > f7b422b17ee5ee David Howells 2006-06-09 1063 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx
