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

[Christian MAUDERER]

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

Note that both tickets (5 and 6) are clones of an old 4.11 ticket:

   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?

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
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/