From: Jan Kara <jack@suse.cz>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/2] fs: update atime before I/O in generic_file_read_iter
Date: Tue, 4 Oct 2016 10:13:24 +0200 [thread overview]
Message-ID: <20161004081324.GC17515@quack2.suse.cz> (raw)
In-Reply-To: <1475225194-3702-3-git-send-email-hch@lst.de>
On Fri 30-09-16 10:46:34, Christoph Hellwig wrote:
> After the call to ->direct_IO the final reference to the file might have
> been dropped by aio_complete already, and the call to file_accessed might
> cause a use after free.
>
> Instead update the access time before the I/O, similar to how we
> update the time stamps before writes.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
But frankly it looks like a nasty catch that iocb->ki_filp can go away
under you in the AIO case. Do I get it right that this means there must be
some other thread closing your fd while the read is running, right?
Also it seems that file_end_write(file) call in aio_run_iocb() is prone to
the same race?
Won't we be better off to just to do additional get_file() / fput() pair in
the AIO submission path so that whole AIO submission path is guaranteed to
have struct file available? I understand this is very performance sensitive
path but we'll be adding just two atomic ops...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2016-10-04 8:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-30 8:46 fix struct file use after free in the AIO read path Christoph Hellwig
2016-09-30 8:46 ` [PATCH 1/2] xfs: update atime before I/O in xfs_file_dio_aio_read Christoph Hellwig
2016-09-30 8:46 ` [PATCH 2/2] fs: update atime before I/O in generic_file_read_iter Christoph Hellwig
2016-10-04 8:13 ` Jan Kara [this message]
2016-10-04 11:48 ` Christoph Hellwig
2016-10-05 8:29 ` Jan Kara
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=20161004081324.GC17515@quack2.suse.cz \
--to=jack@suse.cz \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.