[kbuild] Re: [PATCH v3 07/15] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info
I think my bug report was not clear... :/ The code looks like this: sb = inode->i_sb; if (inode) ... The NULL check cannot be false because if "inode" is NULL we would have already crashed when we dereference it on the line before. In this case, based on last years discussion, the "inode" pointer can't be NULL. The debate is only whether the unnecessary NULL checks help readability or hurt readability. > Why does it presume that event_info->dir is non-NULL? That was my commentary, just from reading the code. Smatch says that "event->dir" is unknown. > Did smach check all the callers to fsnotify() or something? The kbuild-bot doesn't build the cross function database but if you did use the cross function database then, yes, it does track all the callers. There are two pointers that we care about, the "inode" and the parent inode (dir). Smatch can figure out when "inode" is NULL vs non-NULL but where it gets stuck is on the some of the parent inodes like this call from fsnotify_dirent(): fsnotify_name(dir, mask, d_inode(dentry), >d_name, 0); ^^^ Smatch doesn't know that d_inode() is always non-NULL at this point. regards, dan carpenter ___ kbuild mailing list -- kbuild@lists.01.org To unsubscribe send an email to kbuild-le...@lists.01.org
[kbuild] Re: [PATCH v3 07/15] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info
On Wed, Jun 30, 2021 at 11:35:32AM +0300, Amir Goldstein wrote: > > Do you have feeling of dejavu? ;-) > https://lore.kernel.org/linux-fsdevel/20200730192537.gb13...@quack2.suse.cz/ That was a year ago. I have trouble remembering emails I sent yesterday. > > We've been through this. > Maybe you silenced the smach warning on fsnotify() and the rename to > __fsnotifty() > caused this warning to refloat? Yes. Renaming the function will make it show up as a new warning. Also this is an email from the kbuild-bot and last years email was from me, so it's a different tool and a different record of sent messages. (IMO, you should really just remove the bogus NULL checks because everyone looking at the warning will think the code is buggy). regards, dan carpenter ___ kbuild mailing list -- kbuild@lists.01.org To unsubscribe send an email to kbuild-le...@lists.01.org
[kbuild] Re: [PATCH v3 07/15] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info
CC: kbuild-...@lists.01.org
In-Reply-To: <20210629191035.681913-8-kris...@collabora.com>
References: <20210629191035.681913-8-kris...@collabora.com>
TO: Gabriel Krisman Bertazi
TO: amir7...@gmail.com
CC: djw...@kernel.org
CC: ty...@mit.edu
CC: da...@fromorbit.com
CC: j...@suse.com
CC: dhowe...@redhat.com
CC: kha...@google.com
CC: linux-fsde...@vger.kernel.org
CC: linux-e...@vger.kernel.org
CC: ker...@collabora.com
Hi Gabriel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on ext3/fsnotify]
[also build test WARNING on ext4/dev linus/master v5.13 next-20210629]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210630-031347
base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git
fsnotify
:: branch date: 4 hours ago
:: commit date: 4 hours ago
config: x86_64-randconfig-m001-20210628 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
fs/notify/fsnotify.c:505 __fsnotify() warn: variable dereferenced before check
'inode' (see line 494)
vim +/inode +505 fs/notify/fsnotify.c
d9a6f30bb89309 Amir Goldstein 2018-04-20 448
90586523eb4b34 Eric Paris 2009-05-21 449 /*
40a100d3adc1ad Amir Goldstein 2020-07-22 450 * fsnotify - This is
the main call to fsnotify.
40a100d3adc1ad Amir Goldstein 2020-07-22 451 *
40a100d3adc1ad Amir Goldstein 2020-07-22 452 * The VFS calls into
hook specific functions in linux/fsnotify.h.
40a100d3adc1ad Amir Goldstein 2020-07-22 453 * Those functions then
in turn call here. Here will call out to all of the
40a100d3adc1ad Amir Goldstein 2020-07-22 454 * registered
fsnotify_group. Those groups can then use the notification event
40a100d3adc1ad Amir Goldstein 2020-07-22 455 * in whatever means
they feel necessary.
40a100d3adc1ad Amir Goldstein 2020-07-22 456 *
40a100d3adc1ad Amir Goldstein 2020-07-22 457 * @mask:event
type and flags
dca640915c7b84 Amir Goldstein 2021-06-29 458 * Input args in struct
fsnotify_event_info:
40a100d3adc1ad Amir Goldstein 2020-07-22 459 * @data:object
that event happened on
40a100d3adc1ad Amir Goldstein 2020-07-22 460 * @data_type: type of
object for fanotify_data_XXX() accessors
40a100d3adc1ad Amir Goldstein 2020-07-22 461 * @dir: optional
directory associated with event -
dca640915c7b84 Amir Goldstein 2021-06-29 462 * if
@name is not NULL, this is the directory that
dca640915c7b84 Amir Goldstein 2021-06-29 463 * @name
is relative to
dca640915c7b84 Amir Goldstein 2021-06-29 464 * @name:
optional file name associated with event
40a100d3adc1ad Amir Goldstein 2020-07-22 465 * @inode:
optional inode associated with event -
40a100d3adc1ad Amir Goldstein 2020-07-22 466 * either
@dir or @inode must be non-NULL.
40a100d3adc1ad Amir Goldstein 2020-07-22 467 * if both
are non-NULL event may be reported to both.
40a100d3adc1ad Amir Goldstein 2020-07-22 468 * @cookie: inotify
rename cookie
90586523eb4b34 Eric Paris 2009-05-21 469 */
dca640915c7b84 Amir Goldstein 2021-06-29 470 int __fsnotify(__u32
mask, const struct fsnotify_event_info *event_info)
90586523eb4b34 Eric Paris 2009-05-21 471 {
dca640915c7b84 Amir Goldstein 2021-06-29 472 const struct path *path
= fsnotify_event_info_path(event_info);
dca640915c7b84 Amir Goldstein 2021-06-29 473 struct inode *inode =
event_info->inode;
3427ce71554123 Miklos Szeredi 2017-10-30 474 struct
fsnotify_iter_info iter_info = {};
40a100d3adc1ad Amir Goldstein 2020-07-22 475 struct super_block *sb;
60f7ed8c7c4d06 Amir Goldstein 2018-09-01 476 struct mount *mnt =
NULL;
fecc4559780d52 Amir Goldstein 2020-12-02 477 struct inode *parent =
NULL;
9385a84d7e1f65 Jan Kara 2016-11-10 478 int ret = 0;
71d734103edfa2 Mel Gorman 2020-07-08 479 __u32 test_mask,
marks_mask;
90586523eb4b34 Eric Paris 2009-05-21 480
71d734103edfa2 Mel Gorman 2020-07-08 481 if (path)
aa93bdc5500cc9 Amir Goldstein 2020-03-19 482 mnt =
real_mount(path->mnt);
3a9fb89f4cd04c Eric Paris 2009-12-17 483
40a100d3adc1ad Amir Goldstein 2020-07-22 484 if (!inode) {
40a100d3adc1ad Amir Goldstein 2020-07-22 485 /* Dirent event
- report on TYPE_INODE to dir */
