[PATCH] imfs: Fix index underrun when extending empty file

Christian MAUDERER christian.mauderer at embedded-brains.de
Mon Apr 4 14:41:56 UTC 2022


Am 04.04.22 um 16:40 schrieb Joel Sherrill:
> 
> 
> On Mon, Apr 4, 2022, 9:28 AM Christian MAUDERER 
> <christian.mauderer at embedded-brains.de 
> <mailto:christian.mauderer at embedded-brains.de>> wrote:
> 
>     Please note: I would like to apply this to the 5 branch too. I
>     created a
>     ticket for 5 here:
> 
>     https://devel.rtems.org/ticket/4638
>     <https://devel.rtems.org/ticket/4638>
> 
>     Note that both tickets (5 and 6) are clones of an old 4.11 ticket:
> 
>     https://devel.rtems.org/ticket/2353
>     <https://devel.rtems.org/ticket/2353>
> 
>     I didn't plan to backport the patch to 4.11. I'll add a comment to the
>     ticket that the problem is fixed in 5 and 6. Should I close the 4.11
>     ticket with a "wontfix" or just let it open?
> 
> 
> If the ticket applies easily, just apply it after testing on 5 and 6. 
> Please.
> 

OK. I'll try it.

> Is there a test case for this?

The patch adds one.

> 
> --joel
> 
> 
>     Best regards
> 
>     Christian
> 
> 
>     Am 04.04.22 um 16:23 schrieb Christian Mauderer:
>      > Currently the following sequence causes a endless loop when
>     extending an
>      > IMFS file:
>      >
>      > - Create a file with zero length and close it.
>      > - Make sure nearly no allocatable memory is left.
>      > - Open the file and write enough data into it that more than the
>      >    remaining memory will be used.
>      >
>      > In that case when extending the IMFS file, the file currently
>     need zero
>      > blocks. If allocating enough new blocks fails, the already
>     allocated new
>      > blocks will be freed again.
>      >
>      > The comparison of block>=old_blocks that has been used prior to this
>      > patch compared two unsigned numbers. If old_blocks was zero, the
>      > comparison of these two numbers always evaluated to true.
>      >
>      > This patch frees the last block in a separate step to avoid this
>      > problem.
>      >
>      > Fixes #4639
>      > ---
>      >   cpukit/libfs/src/imfs/imfs_memfile.c |  3 ++-
>      >   testsuites/psxtests/psximfs02/init.c | 30
>     +++++++++++++++++++++++++++-
>      >   2 files changed, 31 insertions(+), 2 deletions(-)
>      >
>      > diff --git a/cpukit/libfs/src/imfs/imfs_memfile.c
>     b/cpukit/libfs/src/imfs/imfs_memfile.c
>      > index 23c7192717..769a570ecf 100644
>      > --- a/cpukit/libfs/src/imfs/imfs_memfile.c
>      > +++ b/cpukit/libfs/src/imfs/imfs_memfile.c
>      > @@ -208,9 +208,10 @@ static int IMFS_memfile_extend(
>      >             offset = 0;
>      >          }
>      >       } else {
>      > -       for ( ; block>=old_blocks ; block-- ) {
>      > +       for ( ; block>old_blocks ; block-- ) {
>      >            IMFS_memfile_remove_block( memfile, block );
>      >          }
>      > +       IMFS_memfile_remove_block( memfile, old_blocks );
>      >          rtems_set_errno_and_return_minus_one( ENOSPC );
>      >       }
>      >     }
>      > diff --git a/testsuites/psxtests/psximfs02/init.c
>     b/testsuites/psxtests/psximfs02/init.c
>      > index 15b9137121..04f806f565 100644
>      > --- a/testsuites/psxtests/psximfs02/init.c
>      > +++ b/testsuites/psxtests/psximfs02/init.c
>      > @@ -23,6 +23,8 @@
>      >   #include <rtems/malloc.h>
>      >   #include <rtems/libcsupport.h>
>      >
>      > +#define MEMFILE_BYTES_PER_BLOCK 16
>      > +
>      >   const char rtems_test_name[] = "PSXIMFS 2";
>      >
>      >   /* forward declarations to avoid warnings */
>      > @@ -43,12 +45,17 @@ rtems_task Init(
>      >     static const uintptr_t slink_2_name_size [] = {
>      >       sizeof( slink_2_name )
>      >     };
>      > +  static const uintptr_t some_blocks [] = {
>      > +    MEMFILE_BYTES_PER_BLOCK * 10
>      > +  };
>      > +  static const char some_data[MEMFILE_BYTES_PER_BLOCK * 11];
>      >
>      >     int status = 0;
>      >     void *opaque;
>      >     char linkname_n[32] = {0};
>      >     char linkname_p[32] = {0};
>      >     int i;
>      > +  int fd;
>      >     struct stat stat_buf;
>      >
>      >     TEST_BEGIN();
>      > @@ -102,6 +109,27 @@ rtems_task Init(
>      >     rtems_test_assert( status == -1 );
>      >     rtems_test_assert( errno == EACCES );
>      >
>      > +  puts( "Allocate most of heap with a little bit left" );
>      > +  opaque = rtems_heap_greedy_allocate( some_blocks, 1 );
>      > +
>      > +  puts( "Create an empty file.");
>      > +  status = mknod( "/foo", S_IFREG | S_IRWXU, 0LL );
>      > +  rtems_test_assert( status == 0 );
>      > +
>      > +  puts( "Then increase it's size to more than remaining space" );
>      > +  fd = open( "/foo", O_WRONLY | O_TRUNC);
>      > +  rtems_test_assert( fd >= 0 );
>      > +  status = write(fd, some_data, sizeof(some_data));
>      > +  rtems_test_assert( status == -1);
>      > +  rtems_test_assert( errno == ENOSPC );
>      > +
>      > +  puts( "Clean up again" );
>      > +  status = close(fd);
>      > +  rtems_test_assert( status == 0);
>      > +  status = remove( "/foo" );
>      > +  rtems_test_assert( status == 0);
>      > +  rtems_heap_greedy_free( opaque );
>      > +
>      >     puts( "Allocate most of heap" );
>      >     opaque = rtems_heap_greedy_allocate( mount_table_entry_size, 1 );
>      >
>      > @@ -202,7 +230,7 @@ rtems_task Init(
>      >   #define CONFIGURE_FILESYSTEM_IMFS
>      >
>      >   #define CONFIGURE_MAXIMUM_TASKS                  1
>      > -#define CONFIGURE_IMFS_MEMFILE_BYTES_PER_BLOCK   16
>      > +#define CONFIGURE_IMFS_MEMFILE_BYTES_PER_BLOCK 
>       MEMFILE_BYTES_PER_BLOCK
>      >   #define CONFIGURE_IMFS_ENABLE_MKFIFO
>      >   #define CONFIGURE_MAXIMUM_FILE_DESCRIPTORS 4
>      >   #define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
> 
>     -- 
>     --------------------------------------------
>     embedded brains GmbH
>     Herr Christian MAUDERER
>     Dornierstr. 4
>     82178 Puchheim
>     Germany
>     email: christian.mauderer at embedded-brains.de
>     <mailto:christian.mauderer at embedded-brains.de>
>     phone: +49-89-18 94 741 - 18
>     fax:   +49-89-18 94 741 - 08
> 
>     Registergericht: Amtsgericht München
>     Registernummer: HRB 157899
>     Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>     Unsere Datenschutzerklärung finden Sie hier:
>     https://embedded-brains.de/datenschutzerklaerung/
>     <https://embedded-brains.de/datenschutzerklaerung/>
>     _______________________________________________
>     devel mailing list
>     devel at rtems.org <mailto:devel at rtems.org>
>     http://lists.rtems.org/mailman/listinfo/devel
>     <http://lists.rtems.org/mailman/listinfo/devel>
> 

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
phone: +49-89-18 94 741 - 18
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


More information about the devel mailing list