[bug report] cifs: cache the dirents for entries in a cached directory

[bug report] cifs: cache the dirents for entries in a cached directory

[Dan Carpenter]

Hello Ronnie Sahlberg,

This is a semi-automatic email about new static checker warnings.

The patch d87c48ce4d89: "cifs: cache the dirents for entries in a
cached directory" from May 10, 2022, leads to the following Smatch
complaint:

    fs/cifs/readdir.c:1108 cifs_readdir()
    warn: variable dereferenced before check 'cfid' (see line 1093)

fs/cifs/readdir.c
  1092		 */
  1093		if (cfid->dirents.is_valid) {
  1094			if (!dir_emit_dots(file, ctx)) {
  1095				mutex_unlock(&cfid->dirents.de_mutex);
  1096				goto rddir2_exit;
  1097			}
  1098			emit_cached_dirents(&cfid->dirents, ctx);
  1099			mutex_unlock(&cfid->dirents.de_mutex);
  1100			goto rddir2_exit;
  1101		}
  1102		mutex_unlock(&cfid->dirents.de_mutex);
                             ^^^^^^^^
The patch introduces these dereferences

  1103	
  1104		/* Drop the cache while calling initiate_cifs_search and
  1105		 * find_cifs_entry in case there will be reconnects during
  1106		 * query_directory.
  1107		 */
  1108		if (cfid) {
                    ^^^^
and the NULL check.

  1109			close_cached_dir(cfid);
  1110			cfid = NULL;

regards,
dan carpenter

Re: [bug report] cifs: cache the dirents for entries in a cached directory

[Steve French]

That should be an unneeded check that causes that - had already fixed
this yesterday though

https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=9f114d7bfc6c35ca23a82efce60e0db535a186f1

On Thu, May 26, 2022 at 1:31 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Ronnie Sahlberg,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch d87c48ce4d89: "cifs: cache the dirents for entries in a
> cached directory" from May 10, 2022, leads to the following Smatch
> complaint:
>
>     fs/cifs/readdir.c:1108 cifs_readdir()
>     warn: variable dereferenced before check 'cfid' (see line 1093)
>
> fs/cifs/readdir.c
>   1092           */
>   1093          if (cfid->dirents.is_valid) {
>   1094                  if (!dir_emit_dots(file, ctx)) {
>   1095                          mutex_unlock(&cfid->dirents.de_mutex);
>   1096                          goto rddir2_exit;
>   1097                  }
>   1098                  emit_cached_dirents(&cfid->dirents, ctx);
>   1099                  mutex_unlock(&cfid->dirents.de_mutex);
>   1100                  goto rddir2_exit;
>   1101          }
>   1102          mutex_unlock(&cfid->dirents.de_mutex);
>                              ^^^^^^^^
> The patch introduces these dereferences
>
>   1103
>   1104          /* Drop the cache while calling initiate_cifs_search and
>   1105           * find_cifs_entry in case there will be reconnects during
>   1106           * query_directory.
>   1107           */
>   1108          if (cfid) {
>                     ^^^^
> and the NULL check.
>
>   1109                  close_cached_dir(cfid);
>   1110                  cfid = NULL;
>
> regards,
> dan carpenter



-- 
Thanks,

Steve