Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Martin Steigerwald <martin@lichtvoll.de>,
	Matthew Wilcox <willy@infradead.org>,
	dsterba@suse.cz, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	linux-m68k@lists.linux-m68k.org,
	Debian m68k <debian-68k@lists.debian.org>
Subject: Re: moving affs + RDB partition support to staging?
Date: Sun, 6 May 2018 21:46:23 +0100
Message-ID: <20180506204622.GL30522@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20180506073955.GJ30522@ZenIV.linux.org.uk>

On Sun, May 06, 2018 at 08:39:55AM +0100, Al Viro wrote:
> On Sun, May 06, 2018 at 01:59:51AM +0100, Al Viro wrote:
> 
> > > There is nothing at the moment that needs fixing.
> > 
> > Funny, that...  I'd been going through the damn thing for the
> > last week or so; open-by-fhandle/nfs export support is completely
> > buggered.  And as for the rest... the least said about the error
> > handling, the better - something like rename() hitting an IO
> > error (read one) can not only screw the on-disk data into the
> > ground, it can do seriously bad things to kernel data structures.
> 
> ... and while we are at it, consider the following:

[snip]

Another piece of fun: in
affs_add_entry() we have

        retval = affs_insert_hash(dir, bh);
        mark_buffer_dirty_inode(bh, inode);
        affs_unlock_dir(dir);
        affs_unlock_link(inode);

	d_instantiate(dentry, inode);
done:
        affs_brelse(inode_bh);
        affs_brelse(bh);
        return retval;

and in its callers - things like
        error = affs_add_entry(dir, inode, dentry, ST_USERDIR);
        if (error) {
                clear_nlink(inode);
                mark_inode_dirty(inode);
                iput(inode);
                return error;
        }

	Guess what happens if we hit affs_insert_hash() failure?
d_instantiate() doesn't do anything to in-core inode refcount -
that's caller's responsibility.  affs_new_inode() has allocated
an inode with ->i_count equal to 1; d_instantiate() transfers that
reference into dentry (as ->d_inode).  And then, since we'd got
a non-zero error we do hit iput() (same as we would've if an error
happened early enough in affs_add_entry() to bypass d_instantiate()).
That drives ->i_count to 0, getting inode freed (zero link count
when the last in-core reference is dropped means that there's no
point retaining it in icache).
	So in that case we end up with struct dentry (still hashed
and available for lookups to pick) that has ->d_inode pointing
to freed memory.  Welcome to memory corruption...

	I'm fixing that pile of crap (along with the NFS exports
one and, hopefully, rename mess as well).  HOWEVER, I am not going
to take over the damn thing - David has violated the 11th
commandment (Thou Shalt Never Volunteer), so he gets to joy of
learning that codebase and taking care of it from now on.

  reply index

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 15:46 Moving unmaintained filesystems to staging Matthew Wilcox
2018-04-25 15:47 ` Christoph Hellwig
2018-04-25 20:30 ` David Sterba
2018-04-26  2:57   ` Matthew Wilcox
2018-04-26 10:28     ` moving affs + RDB partition support to staging? (was: Re: Moving unmaintained filesystems to staging) Martin Steigerwald
2018-04-26 10:45       ` moving affs + RDB partition support to staging? John Paul Adrian Glaubitz
2018-04-26 10:59         ` David Sterba
2018-04-26 11:06           ` John Paul Adrian Glaubitz
2018-05-06  0:59         ` Al Viro
2018-05-06  7:40           ` Al Viro
2018-05-06 20:46             ` Al Viro [this message]
2018-05-06 20:49               ` John Paul Adrian Glaubitz
2018-05-06 21:32               ` Al Viro
2018-05-07  2:15                 ` Al Viro
2018-05-07  2:40                   ` Michael Schmitz
2018-05-07  7:08                     ` Martin Steigerwald
2018-05-07 20:50                       ` Michael Schmitz
2018-05-07 20:56                         ` Ingo Jürgensmann
2018-05-07 20:58                           ` John Paul Adrian Glaubitz
2018-05-06  8:40           ` John Paul Adrian Glaubitz
2018-05-06 10:12           ` Martin Steigerwald
2018-04-26 11:00       ` moving affs + RDB partition support to staging? (was: Re: Moving unmaintained filesystems to staging) Christoph Hellwig
2018-04-26 11:08       ` Geert Uytterhoeven
2018-04-26 23:56         ` Finn Thain
2018-04-27  1:43           ` moving affs + RDB partition support to staging? jdow
2018-04-27  1:26         ` jdow
2018-05-06  8:52           ` John Paul Adrian Glaubitz
2018-05-06 10:10             ` Martin Steigerwald
2018-05-07  4:54             ` jdow
2018-04-27  2:11         ` moving affs + RDB partition support to staging? (was: Re: Moving unmaintained filesystems to staging) Michael Schmitz
2018-06-24  9:06           ` Martin Steigerwald
2018-06-24 11:33             ` moving affs + RDB partition support to staging? jdow
2018-06-24 11:40             ` jdow
2018-06-26  2:23               ` Michael Schmitz
2018-06-26  5:17                 ` jdow
2018-06-26  8:12                   ` Martin Steigerwald
2018-06-26  9:46                     ` jdow
2018-06-26  8:31                   ` Michael Schmitz
2018-06-26  9:45                     ` jdow
2018-06-27  1:07                       ` Michael Schmitz
2018-06-27  6:24                         ` jdow
2018-06-27  8:03                           ` Martin Steigerwald
2018-06-28  2:57                             ` jdow
2018-06-28  7:40                               ` Amiga RDB partition support for disks >= 2 TB (was: Re: moving affs + RDB partition support to staging?) Martin Steigerwald
2018-06-27  9:00                           ` moving affs + RDB partition support to staging? Michael Schmitz
2018-06-28  3:44                             ` jdow
2018-06-28  5:43                               ` Michael Schmitz
2018-06-28  6:39                                 ` jdow
2018-06-28  8:16                                   ` Amiga RDB partition support for disks >= 2 TB (was: Re: moving affs + RDB partition support to staging?) Martin Steigerwald
2018-06-28 10:00                                     ` Amiga RDB partition support for disks >= 2 TB jdow
2018-06-28 11:30                                       ` Martin Steigerwald
2018-06-28 11:38                                         ` Martin Steigerwald
2018-06-28 12:31                                           ` jdow
2018-06-28  8:07                                 ` Amiga RDB partition support for disks >= 2 TB (was: Re: moving affs + RDB partition support to staging?) Martin Steigerwald
2018-06-27  7:57                         ` moving affs + RDB partition support to staging? Martin Steigerwald
2018-06-28  2:56                           ` jdow
2018-06-26  8:02                 ` Martin Steigerwald
2018-06-26  8:40                   ` Michael Schmitz
2018-06-26  9:31                   ` jdow
2018-06-25  7:53             ` moving affs + RDB partition support to staging? (was: Re: Moving unmaintained filesystems to staging) Michael Schmitz
2018-06-25  8:26               ` Martin Steigerwald
2018-06-25  8:40               ` Geert Uytterhoeven
2018-04-27  8:01         ` Martin Steigerwald
2018-04-26  4:58   ` Moving unmaintained filesystems to staging Nikolay Borisov
2018-04-26  5:30     ` Willy Tarreau
2018-04-26  6:11 ` Pavel Machek
2018-04-26 10:36   ` Martin Steigerwald
2018-05-03  9:18     ` Pavel Machek
2018-04-27  1:10   ` Luis R. Rodriguez
2018-04-29 12:07   ` Greg KH
2018-04-29 20:07     ` Ondrej Zary
2018-04-29 23:37       ` Greg KH
2018-05-01 10:14         ` Pavel Machek

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=20180506204622.GL30522@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=axboe@kernel.dk \
    --cc=debian-68k@lists.debian.org \
    --cc=dsterba@suse.cz \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=martin@lichtvoll.de \
    --cc=willy@infradead.org \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git