Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-02-12 Thread Jiri Kosina
On Fri, 1 Feb 2019, Dave Chinner wrote: > So, I'll invite the incoherent, incandescent O_DIRECT rage flames of > Linus to be unleashed again and point out the /other reference/ to > IOCB_NOWAIT in mm/filemap.c. That is, in generic_file_read_iter(), > in the *generic O_DIRECT read path*: > >

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Linus Torvalds
On Thu, Jan 31, 2019 at 11:05 PM Linus Torvalds wrote: > > And part of "best effort" is very much "not a security information leak". Side note: it's entirely possible that the preadv2(RWF_NOWAIT) interface is actually already effectively too slow to be effectively used as much of an attack

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Linus Torvalds
On Thu, Jan 31, 2019 at 9:16 PM Dave Chinner wrote: > > You are conflating "best effort non-blocking operation" with > "atomic guarantee". RWF_NOWAIT/IOCB_NOWAIT is the > former, not the latter. Right. That's my *point*, Dave. It's not 'atomic guarantee", and never will be. We are in 100%

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Dave Chinner
On Thu, Jan 31, 2019 at 09:54:16AM -0800, Linus Torvalds wrote: > On Thu, Jan 31, 2019 at 2:23 AM Michal Hocko wrote: > > > > OK, I guess my question was not precise. What does prevent taking fs > > locks down the path? > > IOCB_NOWAIT has never meant that, and will never mean it. I think

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Dave Chinner
On Thu, Jan 31, 2019 at 10:56:44AM +0100, Michal Hocko wrote: > [Cc fs-devel] > > On Wed 30-01-19 13:44:19, Vlastimil Babka wrote: > > From: Jiri Kosina > > > > preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache > > contents, as > > it reveals metadata about residency of pages

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Linus Torvalds
On Thu, Jan 31, 2019 at 2:23 AM Michal Hocko wrote: > > OK, I guess my question was not precise. What does prevent taking fs > locks down the path? IOCB_NOWAIT has never meant that, and will never mean it. We will never give user space those kinds of guarantees. We do locking for various

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Daniel Gruss
On 1/31/19 1:08 PM, Jiri Kosina wrote: > On Thu, 31 Jan 2019, Daniel Gruss wrote: > >> If I understood it correctly, this patch just removes the advantages of >> preadv2 over mmmap+access for the attacker. > > Which is the desired effect. We are not trying to solve the timing aspect, > as I

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Jiri Kosina
On Thu, 31 Jan 2019, Daniel Gruss wrote: > If I understood it correctly, this patch just removes the advantages of > preadv2 over mmmap+access for the attacker. Which is the desired effect. We are not trying to solve the timing aspect, as I don't think there is a reasonable way to do it, is

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Vlastimil Babka
On 1/31/19 1:04 PM, Daniel Gruss wrote: > On 1/30/19 1:44 PM, Vlastimil Babka wrote: >> Close that sidechannel by always initiating readahead on the cache if we >> encounter a cache miss for preadv2(RWF_NOWAIT); with that in place, probing >> the pagecache residency itself will actually populate

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Daniel Gruss
On 1/30/19 1:44 PM, Vlastimil Babka wrote: > Close that sidechannel by always initiating readahead on the cache if we > encounter a cache miss for preadv2(RWF_NOWAIT); with that in place, probing > the pagecache residency itself will actually populate the cache, making the > sidechannel useless.

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Jiri Kosina
On Thu, 31 Jan 2019, Florian Weimer wrote: > >> I think this needs to use a different flag because the semantics are so > >> much different. If I understand this change correctly, previously, > >> RWF_NOWAIT essentially avoided any I/O, and now it does not. > > > > It still avoid synchronous

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Michal Hocko
On Thu 31-01-19 11:30:24, Jiri Kosina wrote: > On Thu, 31 Jan 2019, Michal Hocko wrote: > > > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > > > > index 9f5e323e883e..7bcdd36e629d 100644 > > > > > --- a/mm/filemap.c > > > > > +++ b/mm/filemap.c > > > > > @@ -2075,8 +2075,6 @@ static ssize_t

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Florian Weimer
* Jiri Kosina: > On Wed, 30 Jan 2019, Florian Weimer wrote: > >> > preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache >> > contents, as it reveals metadata about residency of pages in >> > pagecache. >> > >> > If preadv2(RWF_NOWAIT) returns immediately, it provides a clear "page

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Jiri Kosina
On Thu, 31 Jan 2019, Michal Hocko wrote: > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > > > index 9f5e323e883e..7bcdd36e629d 100644 > > > > --- a/mm/filemap.c > > > > +++ b/mm/filemap.c > > > > @@ -2075,8 +2075,6 @@ static ssize_t generic_file_buffered_read(struct > > > > kiocb *iocb, > >

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Michal Hocko
On Thu 31-01-19 11:15:28, Jiri Kosina wrote: > On Thu, 31 Jan 2019, Michal Hocko wrote: > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > > index 9f5e323e883e..7bcdd36e629d 100644 > > > --- a/mm/filemap.c > > > +++ b/mm/filemap.c > > > @@ -2075,8 +2075,6 @@ static ssize_t

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Jiri Kosina
On Thu, 31 Jan 2019, Michal Hocko wrote: > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 9f5e323e883e..7bcdd36e629d 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -2075,8 +2075,6 @@ static ssize_t generic_file_buffered_read(struct > > kiocb *iocb, > > > > page =

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-31 Thread Michal Hocko
[Cc fs-devel] On Wed 30-01-19 13:44:19, Vlastimil Babka wrote: > From: Jiri Kosina > > preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache contents, > as > it reveals metadata about residency of pages in pagecache. > > If preadv2(RWF_NOWAIT) returns immediately, it provides a

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-30 Thread Jiri Kosina
On Wed, 30 Jan 2019, Florian Weimer wrote: > > preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache > > contents, as it reveals metadata about residency of pages in > > pagecache. > > > > If preadv2(RWF_NOWAIT) returns immediately, it provides a clear "page > > not resident"

Re: [PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-30 Thread Florian Weimer
* Vlastimil Babka: > preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache > contents, as it reveals metadata about residency of pages in > pagecache. > > If preadv2(RWF_NOWAIT) returns immediately, it provides a clear "page > not resident" information, and vice versa. > > Close

[PATCH 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O

2019-01-30 Thread Vlastimil Babka
From: Jiri Kosina preadv2(RWF_NOWAIT) can be used to open a side-channel to pagecache contents, as it reveals metadata about residency of pages in pagecache. If preadv2(RWF_NOWAIT) returns immediately, it provides a clear "page not resident" information, and vice versa. Close that sidechannel