Re: [PATCH] btrfs: fix invalid memory access with journal_info

[David Sterba <dsterba@suse.cz>]

On Wed, May 09, 2018 at 09:53:03PM -0700, Omar Sandoval wrote:
> On Wed, May 09, 2018 at 06:35:25PM +0800, robbieko wrote:
> > From: Robbie Ko <robbieko@synology.com>
> > 
> > When send process requires memory allocation, shrinker may be triggered
> > due to insufficient memory.
> > Then evict_inode gets called when inode is freed, and this function
> > may need to start transaction.
> > However, the journal_info is already points to BTRFS_SEND_TRANS_STUB, it
> > passed the if condition,
> > and the following use yields illegal memory access.
> > 
> >   if (current->journal_info) {
> >       WARN_ON(type & TRANS_EXTWRITERS);
> >       h = current->journal_info;
> >       refcount_inc(&h->use_count);
> >       WARN_ON(refcount_read(&h->use_count) > 2);
> >       h->orig_rsv = h->block_rsv;
> >       h->block_rsv = NULL;
> >       goto got_it;
> >   }
> 
> start_transaction() has
> 
>     ASSERT(current->journal_info != BTRFS_SEND_TRANS_STUB);
> 
> Are you saying that's wrong? Are there other cases where the shrinker
> can end up starting a transaction?
> 
> > Direct IO has a similar problem, journal_info will store btrfs_dio_data,
> > which will lead to illegal memory access.
> 
> I have patches getting rid of this for direct I/O here:
> https://github.com/osandov/linux/tree/btrfs-journal-info-abuse
> 
> I originally did that for btrfs swapfile support, but if it actually
> fixes an existing bug it should be easy to get merged.

Yes please.