[PATCH rtems] dosfs: Fix memory leak on failed mounts.

[Christian Mauderer]

PS: Is this a candidate for a backport to 5? How many tickets would I
have to create for it? One for 5 and one for 6?

On 03/08/2020 15:16, Christian Mauderer wrote:
> Currently if mount fails, a converter isn't destroyed. We have to take
> care of two cases:
> 
> 1. The user doesn't provide a converter.
> 
> In this case mounting a dosfs creates a default converter. This patch
> makes sure that the converter is destroyed again if mount failes for
> this case.
> 
> 2. The user provides a converter.
> 
> In this case it's not sure that the dosfs specific routines are reached
> because mount can fail before that. Therefore the user has to destroy
> the converter himself again. This patch adds a documentation for that
> and implements it in the media server.
> ---
>  cpukit/include/rtems/dosfs.h        | 7 +++++++
>  cpukit/libblock/src/media.c         | 1 +
>  cpukit/libfs/src/dosfs/msdos_init.c | 5 +++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/cpukit/include/rtems/dosfs.h b/cpukit/include/rtems/dosfs.h
> index 7691ed7e43..a79b00720a 100644
> --- a/cpukit/include/rtems/dosfs.h
> +++ b/cpukit/include/rtems/dosfs.h
> @@ -214,6 +214,9 @@ typedef struct {
>    /**
>     * @brief Converter implementation for new file system instance.
>     *
> +   * Note: If you pass a converter to mount, you have to destroy it yourself if
> +   * mount failed. In a good case it is destroyed at unmount.
> +   *
>     * Before converters have been added to the RTEMS implementation of the FAT
>     * file system, the implementation was:
>     * - Short names were saved in code page format (as is still the case).
> @@ -270,6 +273,10 @@ typedef struct {
>     *       RTEMS_FILESYSTEM_READ_WRITE,
>     *       &mount_opts
>     *     );
> +   *
> +   *     if (rv != 0) {
> +   *       mount_opts.converter->handler->destroy(mount_opts.converter);
> +   *     }
>     *   } else {
>     *     rv = -1;
>     *     errno = ENOMEM;
> diff --git a/cpukit/libblock/src/media.c b/cpukit/libblock/src/media.c
> index 5b2b06b5b2..2964c19881 100644
> --- a/cpukit/libblock/src/media.c
> +++ b/cpukit/libblock/src/media.c
> @@ -504,6 +504,7 @@ static rtems_status_code mount_worker(
>      if (rv != 0) {
>        rmdir(mount_path);
>        free(mount_path);
> +      mount_options.converter->handler->destroy(mount_options.converter);
>  
>        return RTEMS_IO_ERROR;
>      }
> diff --git a/cpukit/libfs/src/dosfs/msdos_init.c b/cpukit/libfs/src/dosfs/msdos_init.c
> index dc9c76437d..0649258fa7 100644
> --- a/cpukit/libfs/src/dosfs/msdos_init.c
> +++ b/cpukit/libfs/src/dosfs/msdos_init.c
> @@ -102,10 +102,12 @@ int rtems_dosfs_initialize(
>      int                                rc = 0;
>      const rtems_dosfs_mount_options   *mount_options = data;
>      rtems_dosfs_convert_control       *converter;
> +    bool                               converter_created = false;
>  
>  
>      if (mount_options == NULL || mount_options->converter == NULL) {
>          converter = rtems_dosfs_create_default_converter();
> +        converter_created = true;
>      } else {
>          converter = mount_options->converter;
>      }
> @@ -116,6 +118,9 @@ int rtems_dosfs_initialize(
>                                        &msdos_file_handlers,
>                                        &msdos_dir_handlers,
>                                        converter);
> +        if (rc != 0 && converter_created) {
> +            converter->handler->destroy(converter);
> +        }
>      } else {
>          errno = ENOMEM;
>          rc = -1;
> 

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.