fs/nilfs2/dir.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
Syzbot report a task hung in vcs_open.
When rec_len too small in nilfs_check_folio, it can result in a huge flood
of messages being sent to the console. It eventually caused tty to hung when
retrieving the console_lock().
Reported-by: syzbot+8a192e8d090fa9a31135@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8a192e8d090fa9a31135
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
fs/nilfs2/dir.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index fe5b1a30c509..0a89dda75414 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -32,6 +32,7 @@
#include <linux/pagemap.h>
#include "nilfs.h"
#include "page.h"
+#include <linux/ratelimit.h>
static inline unsigned int nilfs_rec_len_from_disk(__le16 dlen)
{
@@ -115,6 +116,7 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
size_t limit = folio_size(folio);
struct nilfs_dir_entry *p;
char *error;
+ static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL * 5, 1);
if (dir->i_size < folio_pos(folio) + limit) {
limit = dir->i_size - folio_pos(folio);
@@ -148,9 +150,11 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
/* Too bad, we had an error */
Ebadsize:
- nilfs_error(sb,
- "size of directory #%lu is not a multiple of chunk size",
- dir->i_ino);
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
+ "size of directory #%lu is not a multiple of chunk size",
+ dir->i_ino);
+ }
goto fail;
Eshort:
error = "rec_len is smaller than minimal";
@@ -167,18 +171,22 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
Einumber:
error = "disallowed inode number";
bad_entry:
- nilfs_error(sb,
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
"bad entry in directory #%lu: %s - offset=%lu, inode=%lu, rec_len=%zd, name_len=%d",
dir->i_ino, error, (folio->index << PAGE_SHIFT) + offs,
(unsigned long)le64_to_cpu(p->inode),
rec_len, p->name_len);
+ }
goto fail;
Eend:
p = (struct nilfs_dir_entry *)(kaddr + offs);
- nilfs_error(sb,
- "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
- dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
- (unsigned long)le64_to_cpu(p->inode));
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
+ "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
+ dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
+ (unsigned long)le64_to_cpu(p->inode));
+ }
fail:
return false;
}
--
2.43.0Syzbot report a task hung in vcs_open.
When rec_len too small in nilfs_check_folio, it can result in a huge flood
of messages being sent to the console. It eventually caused tty to hung when
retrieving the console_lock().
Reported-by: syzbot+8a192e8d090fa9a31135@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8a192e8d090fa9a31135
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
fs/nilfs2/dir.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index fe5b1a30c509..0a89dda75414 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -32,6 +32,7 @@
#include <linux/pagemap.h>
#include "nilfs.h"
#include "page.h"
+#include <linux/ratelimit.h>
static inline unsigned int nilfs_rec_len_from_disk(__le16 dlen)
{
@@ -115,6 +116,7 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
size_t limit = folio_size(folio);
struct nilfs_dir_entry *p;
char *error;
+ static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL * 5, 1);
if (dir->i_size < folio_pos(folio) + limit) {
limit = dir->i_size - folio_pos(folio);
@@ -148,9 +150,11 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
/* Too bad, we had an error */
Ebadsize:
- nilfs_error(sb,
- "size of directory #%lu is not a multiple of chunk size",
- dir->i_ino);
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
+ "size of directory #%lu is not a multiple of chunk size",
+ dir->i_ino);
+ }
goto fail;
Eshort:
error = "rec_len is smaller than minimal";
@@ -167,18 +171,22 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
Einumber:
error = "disallowed inode number";
bad_entry:
- nilfs_error(sb,
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
"bad entry in directory #%lu: %s - offset=%lu, inode=%lu, rec_len=%zd, name_len=%d",
dir->i_ino, error, (folio->index << PAGE_SHIFT) + offs,
(unsigned long)le64_to_cpu(p->inode),
rec_len, p->name_len);
+ }
goto fail;
Eend:
p = (struct nilfs_dir_entry *)(kaddr + offs);
- nilfs_error(sb,
- "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
- dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
- (unsigned long)le64_to_cpu(p->inode));
+ if (__ratelimit(&rs)) {
+ nilfs_error(sb,
+ "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
+ dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
+ (unsigned long)le64_to_cpu(p->inode));
+ }
fail:
return false;
}
--
2.43.0On Sat, Sep 28, 2024 at 12:19 AM Lizhi Xu wrote:
>
> Syzbot report a task hung in vcs_open.
> When rec_len too small in nilfs_check_folio, it can result in a huge flood
> of messages being sent to the console. It eventually caused tty to hung when
> retrieving the console_lock().
>
> Reported-by: syzbot+8a192e8d090fa9a31135@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8a192e8d090fa9a31135
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
> fs/nilfs2/dir.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
Thank you for the patch.
I could confirm that the problem is reproducible and that your patch
prevents it, so I will treat this as a nilfs2 side issue.
The patch seems somewhat straightforward, so let me review this a bit
more. I may ask you to make some changes.
Thanks,
Ryusuke Konishi
>
> diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> index fe5b1a30c509..0a89dda75414 100644
> --- a/fs/nilfs2/dir.c
> +++ b/fs/nilfs2/dir.c
> @@ -32,6 +32,7 @@
> #include <linux/pagemap.h>
> #include "nilfs.h"
> #include "page.h"
> +#include <linux/ratelimit.h>
>
> static inline unsigned int nilfs_rec_len_from_disk(__le16 dlen)
> {
> @@ -115,6 +116,7 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> size_t limit = folio_size(folio);
> struct nilfs_dir_entry *p;
> char *error;
> + static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL * 5, 1);
>
> if (dir->i_size < folio_pos(folio) + limit) {
> limit = dir->i_size - folio_pos(folio);
> @@ -148,9 +150,11 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> /* Too bad, we had an error */
>
> Ebadsize:
> - nilfs_error(sb,
> - "size of directory #%lu is not a multiple of chunk size",
> - dir->i_ino);
> + if (__ratelimit(&rs)) {
> + nilfs_error(sb,
> + "size of directory #%lu is not a multiple of chunk size",
> + dir->i_ino);
> + }
> goto fail;
> Eshort:
> error = "rec_len is smaller than minimal";
> @@ -167,18 +171,22 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> Einumber:
> error = "disallowed inode number";
> bad_entry:
> - nilfs_error(sb,
> + if (__ratelimit(&rs)) {
> + nilfs_error(sb,
> "bad entry in directory #%lu: %s - offset=%lu, inode=%lu, rec_len=%zd, name_len=%d",
> dir->i_ino, error, (folio->index << PAGE_SHIFT) + offs,
> (unsigned long)le64_to_cpu(p->inode),
> rec_len, p->name_len);
> + }
> goto fail;
> Eend:
> p = (struct nilfs_dir_entry *)(kaddr + offs);
> - nilfs_error(sb,
> - "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
> - dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
> - (unsigned long)le64_to_cpu(p->inode));
> + if (__ratelimit(&rs)) {
> + nilfs_error(sb,
> + "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
> + dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
> + (unsigned long)le64_to_cpu(p->inode));
> + }
> fail:
> return false;
> }
> --
> 2.43.0
>
On Sat, Sep 28, 2024 at 3:18 AM Ryusuke Konishi wrote:
>
> On Sat, Sep 28, 2024 at 12:19 AM Lizhi Xu wrote:
> >
> > Syzbot report a task hung in vcs_open.
> > When rec_len too small in nilfs_check_folio, it can result in a huge flood
> > of messages being sent to the console. It eventually caused tty to hung when
> > retrieving the console_lock().
> >
> > Reported-by: syzbot+8a192e8d090fa9a31135@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=8a192e8d090fa9a31135
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > ---
> > fs/nilfs2/dir.c | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
>
> Thank you for the patch.
>
> I could confirm that the problem is reproducible and that your patch
> prevents it, so I will treat this as a nilfs2 side issue.
>
> The patch seems somewhat straightforward, so let me review this a bit
> more. I may ask you to make some changes.
>
> Thanks,
> Ryusuke Konishi
Hi Lizhi,
I found that the root cause of this problem is that nilfs_find_entry()
does not abort the search loop even when nilfs_get_folio() returns an
error.
If the i_size of the directory inode is large and the directory is
corrupted, nilfs_find_entry() may continue to loop and output error
messages endlessly in bursts.
Rate-limiting may be able to prevent serial hangs, but it cannot
interrupt the near-endless loop in nilfs_find_entry(), so I don't
think it is the right approach to take to fix the problem.
Like ext2, nilfs_find_entry() should be able to return errors from
nilfs_get_folio() like this:
char *kaddr = nilfs_get_folio(dir, n, foliop);
if (IS_ERR(kaddr))
return ERR_CAST(kaddr);
However, this approach requires some preparation changes so that the
error code returned by nilfs_find_entry() can be propagated to its
callers.
So, would you mind letting me fix this?
Or, if you want to do it yourself, please let me know.
If you refer to the ext2 implementation, you should be able to figure
out how to fix it, even though there are some nilfs-specific
differences.
Thanks,
Ryusuke Konishi
>
> >
> > diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> > index fe5b1a30c509..0a89dda75414 100644
> > --- a/fs/nilfs2/dir.c
> > +++ b/fs/nilfs2/dir.c
> > @@ -32,6 +32,7 @@
> > #include <linux/pagemap.h>
> > #include "nilfs.h"
> > #include "page.h"
> > +#include <linux/ratelimit.h>
> >
> > static inline unsigned int nilfs_rec_len_from_disk(__le16 dlen)
> > {
> > @@ -115,6 +116,7 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> > size_t limit = folio_size(folio);
> > struct nilfs_dir_entry *p;
> > char *error;
> > + static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL * 5, 1);
> >
> > if (dir->i_size < folio_pos(folio) + limit) {
> > limit = dir->i_size - folio_pos(folio);
> > @@ -148,9 +150,11 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> > /* Too bad, we had an error */
> >
> > Ebadsize:
> > - nilfs_error(sb,
> > - "size of directory #%lu is not a multiple of chunk size",
> > - dir->i_ino);
> > + if (__ratelimit(&rs)) {
> > + nilfs_error(sb,
> > + "size of directory #%lu is not a multiple of chunk size",
> > + dir->i_ino);
> > + }
> > goto fail;
> > Eshort:
> > error = "rec_len is smaller than minimal";
> > @@ -167,18 +171,22 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
> > Einumber:
> > error = "disallowed inode number";
> > bad_entry:
> > - nilfs_error(sb,
> > + if (__ratelimit(&rs)) {
> > + nilfs_error(sb,
> > "bad entry in directory #%lu: %s - offset=%lu, inode=%lu, rec_len=%zd, name_len=%d",
> > dir->i_ino, error, (folio->index << PAGE_SHIFT) + offs,
> > (unsigned long)le64_to_cpu(p->inode),
> > rec_len, p->name_len);
> > + }
> > goto fail;
> > Eend:
> > p = (struct nilfs_dir_entry *)(kaddr + offs);
> > - nilfs_error(sb,
> > - "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
> > - dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
> > - (unsigned long)le64_to_cpu(p->inode));
> > + if (__ratelimit(&rs)) {
> > + nilfs_error(sb,
> > + "entry in directory #%lu spans the page boundary offset=%lu, inode=%lu",
> > + dir->i_ino, (folio->index << PAGE_SHIFT) + offs,
> > + (unsigned long)le64_to_cpu(p->inode));
> > + }
> > fail:
> > return false;
> > }
> > --
> > 2.43.0
> >
© 2016 - 2024 Red Hat, Inc.