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

[Gedare Bloom]

On Mon, Aug 3, 2020 at 7:18 AM Christian Mauderer
<christian.mauderer at embedded-brains.de> wrote:
>
> 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?
>

One for each. Anything put on 5 at this point requires a ticket, and
good practice to add one to 6 also, especially for anything that may
affect user behavior. Then a release note should also get added.

a resource leak is a bug worth back-porting, especially with a simple fix.

> 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.
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel