Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <F01D238D-8A6C-4629-ABC5-4A8BAC25951F@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_00BDFCFC-7769-480B-BD04-EE20FF8FE67E";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [RFC][PATCHSET] sorting out RCU-delayed stuff in
 ->destroy_inode()
Date:   Mon, 29 Apr 2019 22:18:04 -0600
In-Reply-To: <20190430030914.GF23075@ZenIV.linux.org.uk>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
To:     Al Viro <viro@zeniv.linux.org.uk>
References: <20190416174900.GT2217@ZenIV.linux.org.uk>
 <CAHk-=wh6cSEztastk6-A0HUSLtJT=9W38xMN5ht-OOAnL80jxg@mail.gmail.com>
 <20190430030914.GF23075@ZenIV.linux.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--Apple-Mail=_00BDFCFC-7769-480B-BD04-EE20FF8FE67E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On Apr 29, 2019, at 9:09 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>=20
> On Tue, Apr 16, 2019 at 11:01:16AM -0700, Linus Torvalds wrote:
>>=20
>> I only skimmed through the actual filesystem (and one networking)
>> patches, but they looked like trivial conversions to a better
>> interface.
>=20
> ... except that this callback can (and always could) get executed =
after
> freeing struct super_block.  So we can't just dereference ->i_sb->s_op
> and expect to survive; the table ->s_op pointed to will still be =
there,
> but ->i_sb might very well have been freed, with all its contents =
overwritten.
> We need to copy the callback into struct inode itself, unfortunately.
> The following incremental fixes it; I'm going to fold it into the =
first
> commit in there.
>=20
> diff --git a/fs/inode.c b/fs/inode.c
> index fb45590d284e..855dad43b11d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -164,6 +164,7 @@ int inode_init_always(struct super_block *sb, =
struct inode *inode)
> 	inode->i_wb_frn_avg_time =3D 0;
> 	inode->i_wb_frn_history =3D 0;
> #endif
> +	inode->free_inode =3D sb->s_op->free_inode;
>=20
> 	if (security_inode_alloc(inode))
> 		goto out;
> @@ -211,8 +212,8 @@ EXPORT_SYMBOL(free_inode_nonrcu);
> static void i_callback(struct rcu_head *head)
> {
> 	struct inode *inode =3D container_of(head, struct inode, i_rcu);
> -	if (inode->i_sb->s_op->free_inode)
> -		inode->i_sb->s_op->free_inode(inode);
> +	if (inode->free_inode)
> +		inode->free_inode(inode);
> 	else
> 		free_inode_nonrcu(inode);
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2e9b9f87caca..5ed6b39e588e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -718,6 +718,7 @@ struct inode {
> #endif
>=20
> 	void			*i_private; /* fs or device private =
pointer */
> +	void (*free_inode)(struct inode *);

It seems like a waste to increase the size of every struct inode just to =
access
a static pointer.  Is this the only place that ->free_inode() is called? =
 Why
not move the ->free_inode() pointer into inode->i_fop->free_inode() so =
that it
is still directly accessible at this point.

Cheers, Andreas


--Apple-Mail=_00BDFCFC-7769-480B-BD04-EE20FF8FE67E
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAlzHzH0ACgkQcqXauRfM
H+ASkQ/+KJdyGCkWq0gVJ+BAkcY8APT+2IghNpsqirVFNXOVmvA1RYQ4oaCSsRpx
0wLFNjoVJXaXJz6YUfsHXRevmUPZIHHm9R02RlY1jO/zGPCCK9OkzermwwDNzXQJ
LzPvlZweXcK0dqGQpzpf1HdN2gatQK4FBtQxsrhNezQ4rF0Ndz4kxdkx4TdefpEz
8rz113O2moFtRGROxkoDz4q3oFwW+mFp+ukJ+2cwCCCgvxNfew3fT0M7feV1WO+F
RpzZYf8KkR9HkHHSuaJHZNqzHmcJ7BSpX0UCCeh6jON5EJCUH/KRKbL8eEPY1Lqs
KYu471FVq6MxksRXXvXpvNV5CiTRpK52DMQCWgPVA/uswFMcIkemoyaclIgy9egj
vhx4nQD3FUHpSP6feWuzvTr3EJnwrGyJUjaQoqwFQ4zc09JlgMSlrv/7AalRwz4w
mBuU3trIyHIFVs60JGqoOaZ/fJqtUe42EBk08Mb/bLWwQH+ExIi+2X4C6kwwD12E
fKTCgrrLXa/X2dkKl1jQ6QKMIGOvr9H9uBpyz+L3GqYswMA6Zu6ENHockKiC4JZU
kTvM+xWOC70OIgeHqg/6QVteQjhCMrAre93kGvBswXAfehDuFiAuD5NHnZRG2gxX
qZZjgZ8D0L2RZQrWT85ZdxI+ynAYVhurZ1Uf+3oG+F319sRH0xw=
=NJT/
-----END PGP SIGNATURE-----

--Apple-Mail=_00BDFCFC-7769-480B-BD04-EE20FF8FE67E--