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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 17 May 2019 13:17:47 -0700
From:   Ira Weiny <ira.weiny@...el.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Theodore Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: Can ext4_break_layouts() ever fail?

On Fri, May 17, 2019 at 11:02:52AM +0200, Jan Kara wrote:
> On Thu 16-05-19 13:56:15, Ira Weiny wrote:
> 
> > It looks to me like it is possible for ext4_break_layouts() to fail if
> > prepare_to_wait_event() sees a pending signal.  Therefore I think this is a bug
> > in ext4 regardless of how I may implement a truncate failure.
> 
> Yes, it's a bug in ext4.
> 
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -5648,6 +5648,8 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
> >                 if (rc) {  
> >                         up_write(&EXT4_I(inode)->i_mmap_sem);
> >                         error = rc;
> > +                       if (orphan)
> > +                               ext4_orphan_del(NULL, inode);
> 
> This isn't quite correct. This would silence the warning but leave the
> inode in on-disk orphan list. That is OK in case of fs-meltdown types of
> failures like IO errors for metadata, aborted journal, or stuff like that.
> But failing ext4_break_layouts() needs to be handled gracefully maintaining
> fs consistency. So you rather need something like:
> 
> 			if (orphan && inode->i_nlink > 0) {
> 				handle_t *handle;
> 
> 				handle = ext4_journal_start(inode,
> 						EXT4_HT_INODE, 3);
> 				if (IS_ERR(handle)) {
> 					ext4_orphan_del(NULL, inode);
> 					goto err_out;
> 				}
> 				ext4_orphan_del(handle, inode);
> 				ext4_journal_stop(handle);
> 			}
>

Thanks!  Unfortunately, even with your suggestion something is still wrong with
my code.

For some reason this does not seem to be "canceling" the truncate completely.
With my test code for FS DAX which fails ext4_break_layout() the file is being
truncated and an application which is writing past that truncation is getting a
SIGBUS.

I don't understand why this is happening because failing here should be
skipping the call to truncate_pagecache() which AFAICT does user unmappings...
So...  I'm still investigating this.

But thanks for confirming this is a bug.  I will get a generic patch out soon.

Thanks,
Ira

Powered by blists - more mailing lists