On Wed, Oct 06, 2021 at 01:34:00PM +0200, Carlos Maiolino wrote:
> On Tue, Oct 05, 2021 at 03:36:53PM -0700, Darrick J. Wong wrote:
> > On Mon, Oct 04, 2021 at 04:11:40PM +0200, Carlos Maiolino wrote:
> > > Attempting to access a mmapp'ed region that does not correspond to the
> > > file results in a SIGBUS, so prevent xfs_io to even attempt to mmap() a
> > > region beyond EOF.
> > >
> > > Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> > > ---
> > >
> > > There is a caveat about this patch though. It is possible to mmap() a
> > > non-existent file region, extent the file to go beyond such region, and run
> > > operations in this mmapped region without such operations triggering a SIGBUS
> > > (excluding the file corruption factor here :). So, I'm not quite sure if it
> > > would be ok to check for this in mmap_f() as this patch does, or create a helper
> > > to check for such condition, and use it on the other operations (mread_f,
> > > mwrite_f, etc). What you folks think?
> >
> > What's the motivation for checking this in userspace? Programs are
> > allowed to set up this (admittedly minimally functional) configuration,
> > or even set it up after the mmap by truncating the file.
>
> 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...
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.
--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
>
