All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	syzbot <syzbot+d222f4b7129379c3d5bc@syzkaller.appspotmail.com>,
	brauner@kernel.org, jack@suse.cz, jlbec@evilplan.org,
	joseph.qi@linux.alibaba.com, linkinjeon@kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	mark@fasheh.com, ocfs2-devel@lists.linux.dev,
	sj1557.seo@samsung.com, syzkaller-bugs@googlegroups.com,
	Chuck Lever <chuck.lever@oracle.com>
Subject: Re: [PATCH for 6.19-rc1] fs: preserve file type in make_bad_inode() unless invalid
Date: Wed, 10 Dec 2025 20:55:17 +0000	[thread overview]
Message-ID: <20251210205517.GZ1712166@ZenIV> (raw)
In-Reply-To: <20251210204338.GY1712166@ZenIV>

On Wed, Dec 10, 2025 at 08:43:38PM +0000, Al Viro wrote:
> On Wed, Dec 10, 2025 at 03:35:31PM +0000, Al Viro wrote:
> > On Wed, Dec 10, 2025 at 11:09:24AM +0100, Mateusz Guzik wrote:
> > > On Wed, Dec 10, 2025 at 10:45 AM Tetsuo Handa
> > > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> > > >
> > > > syzbot is hitting VFS_BUG_ON_INODE(!S_ISDIR(inode->i_mode)) check
> > > > introduced by commit e631df89cd5d ("fs: speed up path lookup with cheaper
> > > > handling of MAY_EXEC"), for make_bad_inode() is blindly changing file type
> > > > to S_IFREG. Since make_bad_inode() might be called after an inode is fully
> > > > constructed, make_bad_inode() should not needlessly change file type.
> > > >
> > > 
> > > ouch
> > > 
> > > So let's say calls to make_bad_inode *after* d_instantiate are unavoidable.
> > 
> > ... and each one is a bug.
> 
> FWIW, I'm very tempted to fold make_bad_inode() into iget_failed().  Other
> callers tend to be either pointless (e.g. ext2_new_inode() after reaching
> fail: label - we only get there if inode has never reached inode hash
> table; make_bad_inode() in there should've been gone for a long time)
> or outright broken.
> 
> There's not a lot of callers, thankfully; I'm going through those at the
> moment, but so far the impression is that we should be able to simply bury
> the damn thing.

While we are at it, 73861970938a "minixfs: Verify inode mode when loading from
disk" that introduced one of those is seriously misguided - sanity check belongs
in V1_minix_iget/V2_minix_iget, and should be handled there the same way we
deal with zero i_nlink.

We really ought to take that function out - as it is, it's an attractive
nuisance...

  reply	other threads:[~2025-12-10 20:54 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-04  0:07 [syzbot] [exfat?] [ocfs2?] kernel BUG in link_path_walk syzbot
2025-12-04  0:46 ` Mateusz Guzik
2025-12-04  1:21   ` syzbot
2025-12-04  7:45     ` Mateusz Guzik
2025-12-04  8:21       ` Al Viro
2025-12-04  8:40         ` Mateusz Guzik
2025-12-10  9:45           ` [PATCH for 6.19-rc1] fs: preserve file type in make_bad_inode() unless invalid Tetsuo Handa
2025-12-10 10:09             ` Jan Kara
2025-12-10 10:24               ` Mateusz Guzik
2025-12-10 21:14                 ` Al Viro
2025-12-10 23:27                   ` Mateusz Guzik
2026-01-06 10:10               ` Tetsuo Handa
2026-01-07  9:36                 ` Jan Kara
2026-01-07  9:53                   ` Tetsuo Handa
2026-01-07 10:15                     ` Jan Kara
2025-12-10 10:09             ` Mateusz Guzik
2025-12-10 15:35               ` Al Viro
2025-12-10 20:43                 ` Al Viro
2025-12-10 20:55                   ` Al Viro [this message]
2025-12-10 21:33                 ` Al Viro
2025-12-04  9:09 ` [syzbot] [exfat?] [ocfs2?] kernel BUG in link_path_walk Mateusz Guzik
2025-12-04 10:13   ` syzbot
2025-12-04 10:15 ` Mateusz Guzik
2025-12-04 11:56   ` syzbot
2025-12-04 11:58     ` Mateusz Guzik
2025-12-10 11:00 ` Mateusz Guzik
2025-12-10 11:19   ` syzbot
2025-12-10 11:28     ` Mateusz Guzik
2025-12-10 19:44 ` Mateusz Guzik
2025-12-10 20:06   ` syzbot
2025-12-10 21:47   ` Al Viro
2025-12-10 22:09     ` syzbot
2025-12-11  9:00     ` Jan Kara

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=20251210205517.GZ1712166@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=jack@suse.cz \
    --cc=jlbec@evilplan.org \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=mjguzik@gmail.com \
    --cc=ocfs2-devel@lists.linux.dev \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=sj1557.seo@samsung.com \
    --cc=syzbot+d222f4b7129379c3d5bc@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.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.