All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Chao Yu <yuchao0@huawei.com>
Cc: syzbot <syzbot+0eac6f0bbd558fd866d7@syzkaller.appspotmail.com>,
	syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, glider@google.com,
	jaegeuk@kernel.org
Subject: Re: [f2fs-dev] KMSAN: uninit-value in f2fs_lookup
Date: Fri, 25 Sep 2020 13:57:58 +0300	[thread overview]
Message-ID: <20200925105758.GN4282@kadam> (raw)
In-Reply-To: <eb03a5c9-eb77-eb91-e17f-8a3273aab7da@huawei.com>

On Fri, Sep 25, 2020 at 05:06:33PM +0800, Chao Yu wrote:
> Hi,
> 
> I don't see any problem here, thanks for your report. :)
>

I bet the uninitialize value is because "max_depth" is zero.


   352  struct f2fs_dir_entry *__f2fs_find_entry(struct inode *dir,
   353                                           const struct f2fs_filename *fname,
   354                                           struct page **res_page)
                                                               ^^^^^^^^
The stack trace says this isn't initialized.

   355  {
   356          unsigned long npages = dir_blocks(dir);
   357          struct f2fs_dir_entry *de = NULL;
   358          unsigned int max_depth;
   359          unsigned int level;
   360  
   361          if (f2fs_has_inline_dentry(dir)) {
   362                  *res_page = NULL;
   363                  de = f2fs_find_in_inline_dir(dir, fname, res_page);
   364                  goto out;
   365          }
   366  
   367          if (npages == 0) {
   368                  *res_page = NULL;
   369                  goto out;
   370          }
   371  
   372          max_depth = F2FS_I(dir)->i_current_depth;
   373          if (unlikely(max_depth > MAX_DIR_HASH_DEPTH)) {
   374                  f2fs_warn(F2FS_I_SB(dir), "Corrupted max_depth of %lu: %u",
   375                            dir->i_ino, max_depth);
   376                  max_depth = MAX_DIR_HASH_DEPTH;
   377                  f2fs_i_depth_write(dir, max_depth);
   378          }
   379  
   380          for (level = 0; level < max_depth; level++) {
                                ^^^^^^^^^^^^^^^^^
If "max_depth" is zero, then we never enter this loop.

   381                  *res_page = NULL;
   382                  de = find_in_level(dir, level, fname, res_page);
   383                  if (de || IS_ERR(*res_page))
   384                          break;
   385          }
   386  out:
   387          /* This is to increase the speed of f2fs_create */
   388          if (!de)
   389                  F2FS_I(dir)->task = current;
   390          return de;

Which means that we return a NULL "de" and "*res_page" is uninitialized
and that matches what syzbot found throug runtime testing.

   391  }

regards,
dan carpenter


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Chao Yu <yuchao0@huawei.com>
Cc: syzbot <syzbot+0eac6f0bbd558fd866d7@syzkaller.appspotmail.com>,
	chao@kernel.org, glider@google.com, jaegeuk@kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: [f2fs-dev] KMSAN: uninit-value in f2fs_lookup
Date: Fri, 25 Sep 2020 13:57:58 +0300	[thread overview]
Message-ID: <20200925105758.GN4282@kadam> (raw)
In-Reply-To: <eb03a5c9-eb77-eb91-e17f-8a3273aab7da@huawei.com>

On Fri, Sep 25, 2020 at 05:06:33PM +0800, Chao Yu wrote:
> Hi,
> 
> I don't see any problem here, thanks for your report. :)
>

I bet the uninitialize value is because "max_depth" is zero.


   352  struct f2fs_dir_entry *__f2fs_find_entry(struct inode *dir,
   353                                           const struct f2fs_filename *fname,
   354                                           struct page **res_page)
                                                               ^^^^^^^^
The stack trace says this isn't initialized.

   355  {
   356          unsigned long npages = dir_blocks(dir);
   357          struct f2fs_dir_entry *de = NULL;
   358          unsigned int max_depth;
   359          unsigned int level;
   360  
   361          if (f2fs_has_inline_dentry(dir)) {
   362                  *res_page = NULL;
   363                  de = f2fs_find_in_inline_dir(dir, fname, res_page);
   364                  goto out;
   365          }
   366  
   367          if (npages == 0) {
   368                  *res_page = NULL;
   369                  goto out;
   370          }
   371  
   372          max_depth = F2FS_I(dir)->i_current_depth;
   373          if (unlikely(max_depth > MAX_DIR_HASH_DEPTH)) {
   374                  f2fs_warn(F2FS_I_SB(dir), "Corrupted max_depth of %lu: %u",
   375                            dir->i_ino, max_depth);
   376                  max_depth = MAX_DIR_HASH_DEPTH;
   377                  f2fs_i_depth_write(dir, max_depth);
   378          }
   379  
   380          for (level = 0; level < max_depth; level++) {
                                ^^^^^^^^^^^^^^^^^
If "max_depth" is zero, then we never enter this loop.

   381                  *res_page = NULL;
   382                  de = find_in_level(dir, level, fname, res_page);
   383                  if (de || IS_ERR(*res_page))
   384                          break;
   385          }
   386  out:
   387          /* This is to increase the speed of f2fs_create */
   388          if (!de)
   389                  F2FS_I(dir)->task = current;
   390          return de;

Which means that we return a NULL "de" and "*res_page" is uninitialized
and that matches what syzbot found throug runtime testing.

   391  }

regards,
dan carpenter

  reply	other threads:[~2020-09-25 10:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25  5:18 [f2fs-dev] KMSAN: uninit-value in f2fs_lookup syzbot
2020-09-25  5:18 ` syzbot
2020-09-25  9:06 ` [f2fs-dev] " Chao Yu
2020-09-25  9:06   ` Chao Yu
2020-09-25 10:57   ` Dan Carpenter [this message]
2020-09-25 10:57     ` Dan Carpenter
2020-09-25 15:01     ` Chao Yu
2020-09-25 15:01       ` Chao Yu
2020-09-25 16:38   ` Eric Biggers
2020-09-25 16:38     ` Eric Biggers
2020-09-25 16:45     ` Eric Biggers
2020-09-25 16:45       ` Eric Biggers
2020-09-25 23:17       ` Chao Yu
2020-09-25 23:17         ` Chao Yu

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=20200925105758.GN4282@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=glider@google.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+0eac6f0bbd558fd866d7@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=yuchao0@huawei.com \
    /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.