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

Christian MAUDERER christian.mauderer at embedded-brains.de
Thu Apr 7 08:50:32 UTC 2022


Am 04.04.22 um 16:41 schrieb Christian MAUDERER:
> 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.

I applied the patch to all three branches (master, 5, 4.11). I only 
skipped adding the test for 4.11 because that part didn't apply without 
problems. But the bug is fixed there too.

Best regards

Christian

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