| Message ID | 20180526024254.21114-1-lufq.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2018-05-26 at 10:42 +0800, Lu Fengqi wrote: > Creating a file reuses the inode object by kmem_cache_alloc without > initializing maping->wb_err. If it contains an unseen writeback error, > fsync will report an error on the file, even if no error has ever occurred > after the file was created. > > Before commit b4678df184b3 ("errseq: Always report a writeback error > once"), any errors that occur before the file descriptor is opened will be > ignored, causing the issue not to be found. Afterward, any unseen writeback > error will be reported once, including, of course, the uninitialized > maping->wb_err, which exposes this problem. > > Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> > --- > fs/inode.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/inode.c b/fs/inode.c > index 13ceb98c3bd3..bf02dc9d1837 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -182,6 +182,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) > mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE); > mapping->private_data = NULL; > mapping->writeback_index = 0; > + mapping->wb_err = 0; > inode->i_private = NULL; > inode->i_mapping = mapping; > INIT_HLIST_HEAD(&inode->i_dentry); /* buggered by rcu freeing */ (cc'ing Darrick) I think Darrick had a similar patch that also fixed up some related inode reuse issues in xfs. Darrick, were you planning to get that merged for v4.17?
On Sat, May 26, 2018 at 07:42:46AM -0400, Jeff Layton wrote: >On Sat, 2018-05-26 at 10:42 +0800, Lu Fengqi wrote: >> Creating a file reuses the inode object by kmem_cache_alloc without >> initializing maping->wb_err. If it contains an unseen writeback error, >> fsync will report an error on the file, even if no error has ever occurred >> after the file was created. >> >> Before commit b4678df184b3 ("errseq: Always report a writeback error >> once"), any errors that occur before the file descriptor is opened will be >> ignored, causing the issue not to be found. Afterward, any unseen writeback >> error will be reported once, including, of course, the uninitialized >> maping->wb_err, which exposes this problem. >> >> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> >> --- >> fs/inode.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/inode.c b/fs/inode.c >> index 13ceb98c3bd3..bf02dc9d1837 100644 >> --- a/fs/inode.c >> +++ b/fs/inode.c >> @@ -182,6 +182,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) >> mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE); >> mapping->private_data = NULL; >> mapping->writeback_index = 0; >> + mapping->wb_err = 0; >> inode->i_private = NULL; >> inode->i_mapping = mapping; >> INIT_HLIST_HEAD(&inode->i_dentry); /* buggered by rcu freeing */ > >(cc'ing Darrick) > >I think Darrick had a similar patch that also fixed up some related >inode reuse issues in xfs. Darrick, were you planning to get that >merged for v4.17? Oh, I missed Darrick's patch. Thanks for your reminder.
On Sat, May 26, 2018 at 08:29:35PM +0800, Lu Fengqi wrote: > On Sat, May 26, 2018 at 07:42:46AM -0400, Jeff Layton wrote: > >On Sat, 2018-05-26 at 10:42 +0800, Lu Fengqi wrote: > >> Creating a file reuses the inode object by kmem_cache_alloc without > >> initializing maping->wb_err. If it contains an unseen writeback error, > >> fsync will report an error on the file, even if no error has ever occurred > >> after the file was created. > >> > >> Before commit b4678df184b3 ("errseq: Always report a writeback error > >> once"), any errors that occur before the file descriptor is opened will be > >> ignored, causing the issue not to be found. Afterward, any unseen writeback > >> error will be reported once, including, of course, the uninitialized > >> maping->wb_err, which exposes this problem. > >> > >> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> > >> --- > >> fs/inode.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/fs/inode.c b/fs/inode.c > >> index 13ceb98c3bd3..bf02dc9d1837 100644 > >> --- a/fs/inode.c > >> +++ b/fs/inode.c > >> @@ -182,6 +182,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) > >> mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE); > >> mapping->private_data = NULL; > >> mapping->writeback_index = 0; > >> + mapping->wb_err = 0; > >> inode->i_private = NULL; > >> inode->i_mapping = mapping; > >> INIT_HLIST_HEAD(&inode->i_dentry); /* buggered by rcu freeing */ > > > >(cc'ing Darrick) > > > >I think Darrick had a similar patch that also fixed up some related > >inode reuse issues in xfs. Darrick, were you planning to get that > >merged for v4.17? > > Oh, I missed Darrick's patch. Thanks for your reminder. Yeah, it's in my for-next branch now, will send it to Linus as a last minute fix after it's been through the -next wringer (i.e. tomorrow) unless anyone yells. --D > > -- > Thanks, > Lu > > >-- > >Jeff Layton <jlayton@kernel.org> > > > > > >
diff --git a/fs/inode.c b/fs/inode.c index 13ceb98c3bd3..bf02dc9d1837 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -182,6 +182,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE); mapping->private_data = NULL; mapping->writeback_index = 0; + mapping->wb_err = 0; inode->i_private = NULL; inode->i_mapping = mapping; INIT_HLIST_HEAD(&inode->i_dentry); /* buggered by rcu freeing */
Creating a file reuses the inode object by kmem_cache_alloc without initializing maping->wb_err. If it contains an unseen writeback error, fsync will report an error on the file, even if no error has ever occurred after the file was created. Before commit b4678df184b3 ("errseq: Always report a writeback error once"), any errors that occur before the file descriptor is opened will be ignored, causing the issue not to be found. Afterward, any unseen writeback error will be reported once, including, of course, the uninitialized maping->wb_err, which exposes this problem. Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com> --- fs/inode.c | 1 + 1 file changed, 1 insertion(+)