linux-kernel - Re: [PATCH v2] fs: minix: Fix handling of corrupted directories

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6hlyjsiwu5idkqbauvn7ruyztdc5wgkdtkiljk3ncfqfeaanf2@uinqsk54ehww>
Date: Thu, 15 May 2025 12:10:01 +0200
From: Jan Kara <jack@...e.cz>
To: Kitotavrik <kitotavrik.s@...il.com>
Cc: Jan Kara <jack@...e.cz>, Christian Brauner <brauner@...nel.org>, 
	"Matthew Wilcox (Oracle)" <willy@...radead.org>, Josef Bacik <josef@...icpanda.com>, NeilBrown <neilb@...e.de>, 
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org, stable@...r.kernel.org, 
	Andrey Kriulin <kitotavrik.media@...il.com>
Subject: Re: [PATCH v2] fs: minix: Fix handling of corrupted directories

On Wed 14-05-25 22:13:24, Kitotavrik wrote:
> >  guess the easiest is to fudge i_nlinks count in memory to 2 to
> > avoid issues...
> 
> But if a subdirectory was in the corrupted directory(nlinks= 3), it
> will be replaced with nlinks 2. And after deleting subdirectory,
> nlinks was 1 and the problem will remain. Maybe should return EUCLEAN,
> regardless of the mounting mode.

Well, that does not help. You can also corrupt the directory link count so
that it is lower than the number of subdirectories. Then as you delete
subdirectories the parent directory link count is going to get invalid. So
at the places where you decrement parent directory link count (rename,
rmdir) you should check whether the link count doesn't drop below expected
minimum and abort the operation (and print error) if it would.

								Honza

> 
> 
> ср, 14 мая 2025 г. в 18:54, Jan Kara <jack@...e.cz>:
> >
> > On Wed 14-05-25 13:38:35, Andrey Kriulin wrote:
> > > If the directory is corrupted and the number of nlinks is less than 2
> > > (valid nlinks have at least 2), then when the directory is deleted, the
> > > minix_rmdir will try to reduce the nlinks(unsigned int) to a negative
> > > value.
> > >
> > > Make nlinks validity check for directory.
> > >
> > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> > >
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Cc: stable@...r.kernel.org
> > > Signed-off-by: Andrey Kriulin <kitotavrik.media@...il.com>
> > > Signed-off-by: Andrey Kriulin <kitotavrik.s@...il.com>
> > > ---
> > > v2: Move nlinks validaty check to V[12]_minix_iget() per Jan Kara
> > > <jack@...e.cz> request. Change return error code to EUCLEAN. Don't block
> > > directory in r/o mode per Al Viro <viro@...iv.linux.org.uk> request.
> > >
> > >  fs/minix/inode.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> > > index f007e389d5d2..d815397b8b0d 100644
> > > --- a/fs/minix/inode.c
> > > +++ b/fs/minix/inode.c
> > > @@ -517,6 +517,14 @@ static struct inode *V1_minix_iget(struct inode *inode)
> > >               iget_failed(inode);
> > >               return ERR_PTR(-ESTALE);
> > >       }
> > > +     if (S_ISDIR(raw_inode->i_mode) && raw_inode->i_nlinks < 2) {
> > > +             printk("MINIX-fs: inode directory with corrupted number of links");
> >
> > A message like this is rather useless because it shows nothing either about
> > the inode or the link count or the filesystem where this happened. I'd
> > either improve or delete it.
> >
> > > +             if (!sb_rdonly(inode->i_sb)) {
> > > +                     brelse(bh);
> > > +                     iget_failed(inode);
> > > +                     return ERR_PTR(-EUCLEAN);
> > > +             }
> >
> > OK, but when the inode is cached in memory with the wrong link count and
> > then the filesystem is remounted read-write, you will get the same problem
> > as before? I guess the easiest is to fudge i_nlinks count in memory to 2 to
> > avoid issues...
> >
> >
> >                                                                 Honza
> > --
> > Jan Kara <jack@...e.com>
> > SUSE Labs, CR
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR