Re: [PATCH] Prevent mmap command to map beyond EOF

[Carlos Maiolino <cmaiolino@redhat.com>]

> > My biggest motivation was actually seeing xfs_io crashing due a sigbus
> > while running generic/172 and generic/173. And personally, I'd rather see an
> > error message like "attempt to mmap/mwrite beyond EOF" than seeing it crash.
> > Also, as you mentioned, programs are allowed to set up such kind of
> > configuration (IIUC what you mean, mixing mmap, extend, truncate, etc), so, I
> > believe such userspace programs should also ensure they are not attempting to
> > write to invalid memory.
> 
> This patch would /also/ prevent us from writing an fstest to check that
> a process /does/ get SIGBUS when writing to a mapping beyond EOF.  Huh,
> we don't have a test for that...

TBH, this kind of test in xfstests didn't pass through my mind, but, this is
mmap (and its mapped regions) behavior, I don't see why we would want to test
this in xfstests, but, well, I am neither a memory expert to say what it
should/shouldn't do (I'm just following the man page), nor I have any authority
to say what we can/can't do in xfstests :)

> 
> Also, where does generic/173 write to a mapping beyond EOF?  It sets up
> a file of blksz*nr_blks bytes, clones it, fills the fs to full, and then
> writes that number of bytes to the mmap region to trigger SIGBUS when
> the COW fails due to ENOSPC.

The whole command that ends up receiving a SIGBUS is:

xfs_io -i -f -c 'mmap -rw 0 41943040' -c 'mwrite -S 0x62 0 41943040'

I have a slightly suspicious this is happening because it's trying to access the
very last byte of the file, but I do need to check again the machine where I was
running g/173. At a later point, I just did some tests using an empty, 0
sized file. So I do not recall from the top of my head the behavior from g/173.
I can do it later, now I need to run to a doc appt.

> 
> --D
> 
> > > OTOH if your goal is to write a test to check the SIGBUS functionality,
> > > you could install a sigbus handler to report the signal to stderr, which
> > > would avoid bash writing junk about the sigbus to the terminal.
> > 
> > No, I'm just trying to avoid xfs_io crashing if we point it to invalid memory :)
> > 
> > Cheers.
> > 
> > > 
> > > --D
> > > 
> > > > 
> > > >  io/mmap.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/io/mmap.c b/io/mmap.c
> > > > index 9816cf68..77c5f2b6 100644
> > > > --- a/io/mmap.c
> > > > +++ b/io/mmap.c
> > > > @@ -242,6 +242,13 @@ mmap_f(
> > > >  		return 0;
> > > >  	}
> > > >  
> > > > +	/* Check if we are mmapping beyond EOF */
> > > > +	if ((offset + length) > filesize()) {
> > > > +		printf(_("Attempting to mmap() beyond EOF\n"));
> > > > +		exitcode = 1;
> > > > +		return 0;
> > > > +	}
> > > > +
> > > >  	/*
> > > >  	 * mmap and munmap memory area of length2 region is helpful to
> > > >  	 * make a region of extendible free memory. It's generally used
> > > > -- 
> > > > 2.31.1
> > > > 
> > > 
> > 
> > -- 
> > Carlos
> > 
> 

-- 
Carlos