All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: almaz.alexandrovich@paragon-software.com
Cc: ntfs3@lists.linux.dev
Subject: [bug report] fs/ntfs3: Add file operations and implementation
Date: Tue, 24 Aug 2021 12:02:55 +0300	[thread overview]
Message-ID: <20210824090255.GA18518@kili> (raw)

Hello Konstantin Komarov,

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

The patch 4342306f0f0d: "fs/ntfs3: Add file operations and
implementation" from Aug 13, 2021, leads to the following Smatch
complaint:

    fs/ntfs3/namei.c:446 ntfs_rename()
    warn: variable dereferenced before check 'old_inode' (see line 312)

fs/ntfs3/namei.c
   311	
   312		if (ntfs_is_meta_file(sbi, old_inode->i_ino)) {
                                           ^^^^^^^^^^^^^^^^
Dereference

   313			err = -EINVAL;
   314			goto out;
   315		}
   316	
   317		if (new_inode) {
   318			/*target name exists. unlink it*/
   319			dget(new_dentry);
   320			ni_lock_dir(new_dir_ni);
   321			err = ntfs_unlink_inode(new_dir, new_dentry);
   322			ni_unlock(new_dir_ni);
   323			dput(new_dentry);
   324			if (err)
   325				goto out;
   326		}
   327	
   328		/* allocate PATH_MAX bytes */
   329		old_de = __getname();
   330		if (!old_de) {
   331			err = -ENOMEM;
   332			goto out;
   333		}
   334	
   335		err = fill_name_de(sbi, old_de, &old_dentry->d_name, NULL);
   336		if (err < 0)
   337			goto out1;
   338	
   339		old_name = (struct ATTR_FILE_NAME *)(old_de + 1);
   340	
   341		if (is_same) {
   342			new_de = old_de;
   343		} else {
   344			new_de = Add2Ptr(old_de, 1024);
   345			err = fill_name_de(sbi, new_de, &new_dentry->d_name, NULL);
   346			if (err < 0)
   347				goto out1;
   348		}
   349	
   350		ni_lock_dir(old_dir_ni);
   351		ni_lock(old_ni);
   352	
   353		mi_get_ref(&old_dir_ni->mi, &old_name->home);
   354	
   355		/*get pointer to file_name in mft*/
   356		fname = ni_fname_name(old_ni, (struct cpu_str *)&old_name->name_len,
   357				      &old_name->home, &le);
   358		if (!fname) {
   359			err = -EINVAL;
   360			goto out2;
   361		}
   362	
   363		/* Copy fname info from record into new fname */
   364		new_name = (struct ATTR_FILE_NAME *)(new_de + 1);
   365		memcpy(&new_name->dup, &fname->dup, sizeof(fname->dup));
   366	
   367		name_type = paired_name(fname->type);
   368	
   369		/* remove first name from directory */
   370		err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni, old_de + 1,
   371					le16_to_cpu(old_de->key_size), sbi);
   372		if (err)
   373			goto out3;
   374	
   375		/* remove first name from mft */
   376		err = ni_remove_attr_le(old_ni, attr_from_name(fname), le);
   377		if (err)
   378			goto out4;
   379	
   380		le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
   381		old_ni->mi.dirty = true;
   382	
   383		if (name_type != FILE_NAME_POSIX) {
   384			/* get paired name */
   385			fname = ni_fname_type(old_ni, name_type, &le);
   386			if (fname) {
   387				/* remove second name from directory */
   388				err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni,
   389							fname, fname_full_size(fname),
   390							sbi);
   391				if (err)
   392					goto out5;
   393	
   394				/* remove second name from mft */
   395				err = ni_remove_attr_le(old_ni, attr_from_name(fname),
   396							le);
   397				if (err)
   398					goto out6;
   399	
   400				le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
   401				old_ni->mi.dirty = true;
   402			}
   403		}
   404	
   405		/* Add new name */
   406		mi_get_ref(&old_ni->mi, &new_de->ref);
   407		mi_get_ref(&ntfs_i(new_dir)->mi, &new_name->home);
   408	
   409		new_de_key_size = le16_to_cpu(new_de->key_size);
   410	
   411		/* insert new name in mft */
   412		err = ni_insert_resident(old_ni, new_de_key_size, ATTR_NAME, NULL, 0,
   413					 &attr, NULL);
   414		if (err)
   415			goto out7;
   416	
   417		attr->res.flags = RESIDENT_FLAG_INDEXED;
   418	
   419		memcpy(Add2Ptr(attr, SIZEOF_RESIDENT), new_name, new_de_key_size);
   420	
   421		le16_add_cpu(&old_ni->mi.mrec->hard_links, 1);
   422		old_ni->mi.dirty = true;
   423	
   424		/* insert new name in directory */
   425		err = indx_insert_entry(&new_dir_ni->dir, new_dir_ni, new_de, sbi,
   426					NULL);
   427		if (err)
   428			goto out8;
   429	
   430		if (IS_DIRSYNC(new_dir))
   431			err = ntfs_sync_inode(old_inode);
   432		else
   433			mark_inode_dirty(old_inode);
   434	
   435		old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
   436		if (IS_DIRSYNC(old_dir))
   437			(void)ntfs_sync_inode(old_dir);
   438		else
   439			mark_inode_dirty(old_dir);
   440	
   441		if (old_dir != new_dir) {
   442			new_dir->i_mtime = new_dir->i_ctime = old_dir->i_ctime;
   443			mark_inode_dirty(new_dir);
   444		}
   445	
   446		if (old_inode) {
                    ^^^^^^^^^
If old_inode can be NULL we are toasted.

   447			old_inode->i_ctime = old_dir->i_ctime;
   448			mark_inode_dirty(old_inode);

regards,
dan carpenter

             reply	other threads:[~2021-08-24  9:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24  9:02 Dan Carpenter [this message]
2021-08-24  9:32 ` [bug report] fs/ntfs3: Add file operations and implementation Kari Argillander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210824090255.GA18518@kili \
    --to=dan.carpenter@oracle.com \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=ntfs3@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.