[0/5] Fix a minor POSIX conformance problem

  POSIX requires that on ftruncate() expansion, the new bytes must read
as zeroes.  If someone's mmap()ed the file and stored past EOF, for
most filesystems the bytes in that page will be not-zero.  It's a
pretty minor violation; someone could race you and write to the file
between the ftruncate() call and you reading from it, but it's a bit
of a QOI violation.  

I've tested xfs (passes before & after), ext4 and tmpfs (both fail
before, pass after).  Testing from other FS developers appreciated.
fstest to follow; not sure how to persuade git-send-email to work on
multiple repositories

Matthew Wilcox (Oracle) (5):
  truncate: Zero bytes after 'oldsize' if we're expanding the file
  ext4: Zero bytes after 'oldsize' if we're expanding the file
  tmpfs: Zero bytes after 'oldsize' if we're expanding the file
  afs: Zero bytes after 'oldsize' if we're expanding the file
  btrfs: Zero bytes after 'oldsize' if we're expanding the file

 fs/afs/inode.c   | 2 ++
 fs/btrfs/inode.c | 1 +
 fs/ext4/inode.c  | 1 +
 mm/shmem.c       | 2 ++
 mm/truncate.c    | 7 +++++--
 5 files changed, 11 insertions(+), 2 deletions(-)
  

On Feb 2, 2023, at 1:44 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> 
> POSIX requires that on ftruncate() expansion, the new bytes must read
> as zeroes.  If someone's mmap()ed the file and stored past EOF, for
> most filesystems the bytes in that page will be not-zero.  It's a
> pretty minor violation; someone could race you and write to the file
> between the ftruncate() call and you reading from it, but it's a bit
> of a QOI violation.

Is it possible to have mmap return SIGBUS for the writes beyond EOF?
On the one hand, that might indicate incorrect behavior of the application,
and on the other hand, it seems possible that the application doesn't
know it is writing beyond EOF and expects that data to be read back OK?

What happens if it is writing beyond EOF, but the block hasn't even been
allocated because PAGE_SIZE > blocksize?

IMHO, this seems better to stop the root of the problem (mmap() allowing
bad writes), rather than trying to fix it after the fact.

Cheers, Andreas

> I've tested xfs (passes before & after), ext4 and tmpfs (both fail
> before, pass after).  Testing from other FS developers appreciated.
> fstest to follow; not sure how to persuade git-send-email to work on
> multiple repositories
> 
> Matthew Wilcox (Oracle) (5):
>  truncate: Zero bytes after 'oldsize' if we're expanding the file
>  ext4: Zero bytes after 'oldsize' if we're expanding the file
>  tmpfs: Zero bytes after 'oldsize' if we're expanding the file
>  afs: Zero bytes after 'oldsize' if we're expanding the file
>  btrfs: Zero bytes after 'oldsize' if we're expanding the file
> 
> fs/afs/inode.c   | 2 ++
> fs/btrfs/inode.c | 1 +
> fs/ext4/inode.c  | 1 +
> mm/shmem.c       | 2 ++
> mm/truncate.c    | 7 +++++--
> 5 files changed, 11 insertions(+), 2 deletions(-)
> 
> --
> 2.35.1
> 


Cheers, Andreas
  

On Thu, Feb 02, 2023 at 04:08:49PM -0700, Andreas Dilger wrote:
> On Feb 2, 2023, at 1:44 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> > 
> > POSIX requires that on ftruncate() expansion, the new bytes must read
> > as zeroes.  If someone's mmap()ed the file and stored past EOF, for
> > most filesystems the bytes in that page will be not-zero.  It's a
> > pretty minor violation; someone could race you and write to the file
> > between the ftruncate() call and you reading from it, but it's a bit
> > of a QOI violation.
> 
> Is it possible to have mmap return SIGBUS for the writes beyond EOF?

Well, no.  The hardware only tells us about accesses on a per-page
basis.  We could SIGBUS on writes that _start_ after EOF, but this
test doesn't do that (it starts before EOF and extends past EOF).
And once the page is mapped writable, there's no page fault taken
for subsequent writes.

> On the one hand, that might indicate incorrect behavior of the application,
> and on the other hand, it seems possible that the application doesn't
> know it is writing beyond EOF and expects that data to be read back OK?

POSIX says:

"The system shall always zero-fill any partial page at the end of an
object. Further, the system shall never write out any modified portions
of the last page of an object which are beyond its end. References
within the address range starting at pa and continuing for len bytes to
whole pages following the end of an object shall result in delivery of
a SIGBUS signal."

https://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html

So the application can't expect to read back anything it's written
(and if you look at page writeback, we currently zero beyond EOF at
writeback time).

> IMHO, this seems better to stop the root of the problem (mmap() allowing
> bad writes), rather than trying to fix it after the fact.

That would be nice, but we're rather stuck with the hardware that exists.
IIUC Cray-1 had byte-granularity range registers, but page-granularity
is what we have.
  

From: Matthew Wilcox
> Sent: 03 February 2023 13:21
> 
> On Thu, Feb 02, 2023 at 04:08:49PM -0700, Andreas Dilger wrote:
> > On Feb 2, 2023, at 1:44 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> > >
> > > POSIX requires that on ftruncate() expansion, the new bytes must read
> > > as zeroes.  If someone's mmap()ed the file and stored past EOF, for
> > > most filesystems the bytes in that page will be not-zero.  It's a
> > > pretty minor violation; someone could race you and write to the file
> > > between the ftruncate() call and you reading from it, but it's a bit
> > > of a QOI violation.
> >
> > Is it possible to have mmap return SIGBUS for the writes beyond EOF?
> 
> Well, no.  The hardware only tells us about accesses on a per-page
> basis.  We could SIGBUS on writes that _start_ after EOF, but this
> test doesn't do that (it starts before EOF and extends past EOF).
> And once the page is mapped writable, there's no page fault taken
> for subsequent writes.
> 
> > On the one hand, that might indicate incorrect behavior of the application,
> > and on the other hand, it seems possible that the application doesn't
> > know it is writing beyond EOF and expects that data to be read back OK?
> 
> POSIX says:
> 
> "The system shall always zero-fill any partial page at the end of an
> object. Further, the system shall never write out any modified portions
> of the last page of an object which are beyond its end. References
> within the address range starting at pa and continuing for len bytes to
> whole pages following the end of an object shall result in delivery of
> a SIGBUS signal."
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html

It also says (down at the bottom of the rational):

"The mmap() function can be used to map a region of memory that is larger
than the current size of the object. Memory access within the mapping but
beyond the current end of the underlying objects may result in SIGBUS
signals being sent to the process. The reason for this is that the size
of the object can be manipulated by other processes and can change at any
moment. The implementation should tell the application that a memory
reference is outside the object where this can be detected; otherwise,
written data may be lost and read data may not reflect actual data in the
object."

There are a lot of 'may' in that sentence.
Note that it only says that 'data written beyond the current eof may be
lost'.
I think that could be taken to take precedence over the zeroing clause
in ftruncate().
I'd bet a lot of beer that the original SYSV implementation (on with the
description is based) didn't zero the page buffer when ftruncate()
increased the file size.
Whether anything (important) actually relies on that is an interesting
question!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
  

On Fri, Feb 03, 2023 at 04:23:32PM +0000, David Laight wrote:
> From: Matthew Wilcox
> > "The system shall always zero-fill any partial page at the end of an
> > object. Further, the system shall never write out any modified portions
> > of the last page of an object which are beyond its end. References
> > within the address range starting at pa and continuing for len bytes to
> > whole pages following the end of an object shall result in delivery of
> > a SIGBUS signal."
> > 
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html
> 
> It also says (down at the bottom of the rational):
> 
> "The mmap() function can be used to map a region of memory that is larger
> than the current size of the object. Memory access within the mapping but
> beyond the current end of the underlying objects may result in SIGBUS
> signals being sent to the process. The reason for this is that the size
> of the object can be manipulated by other processes and can change at any
> moment. The implementation should tell the application that a memory
> reference is outside the object where this can be detected; otherwise,
> written data may be lost and read data may not reflect actual data in the
> object."
> 
> There are a lot of 'may' in that sentence.
> Note that it only says that 'data written beyond the current eof may be
> lost'.
> I think that could be taken to take precedence over the zeroing clause
> in ftruncate().

How can the _rationale_ (explicitly labelled as informative) for one
function take precedence over the requirements for another function?
This is nonsense.

> I'd bet a lot of beer that the original SYSV implementation (on with the
> description is based) didn't zero the page buffer when ftruncate()
> increased the file size.
> Whether anything (important) actually relies on that is an interesting
> question!
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>