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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 30 Jun 2021 13:49:04 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     kbuild@...ts.01.org,
        Gabriel Krisman Bertazi <krisman@...labora.com>,
        kbuild test robot <lkp@...el.com>, kbuild-all@...ts.01.org,
        "Darrick J. Wong" <djwong@...nel.org>,
        Theodore Tso <tytso@....edu>,
        Dave Chinner <david@...morbit.com>, Jan Kara <jack@...e.com>,
        David Howells <dhowells@...hat.com>,
        Khazhismel Kumykov <khazhy@...gle.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Ext4 <linux-ext4@...r.kernel.org>, kernel@...labora.com
Subject: 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), &dentry->d_name, 0);
                                 ^^^^^^^^^^^^^^^

Smatch doesn't know that d_inode() is always non-NULL at this point.

regards,
dan carpenter

Powered by blists - more mailing lists