nfs-ganesha / nfs-ganesha

NFS-Ganesha is an NFSv3,v4,v4.1 fileserver that runs in user mode on most UNIX/Linux systems
1.51k stars 513 forks source link

Double free gsh_export entry on addexport command #195

Closed SergeyLysanov closed 6 years ago

SergeyLysanov commented 7 years ago

The bug is easy to reproduce. Call dynamic export procedure to config with non existing path: dbus-send --print-reply --system --dest=org.ganesha.nfsd /org/ganesha/nfsd/ExportMgr org.ganesha.nfsd.exportmgr.AddExport string:/root/ganesha.conf string:"export(export_id=77)"

Config sample:

EXPORT
{
  Export_Id = 77;
  Path = /non_existing_path;
  Pseudo = /non_existing_path;
  Access_Type = RW;
  FSAL {
    Name = SOME_fSAL;
  }
}

Ganesha make double free of gsh_export struct. We call twice put_gsh_export(first in export_revert, second in out section in export_commit_common function) and free struct gsh_export. Then call export_init function from proc_bloc and free struct again:

static void *export_init(void *link_mem, void *self_struct)
{
    struct gsh_export *export;

    if (self_struct == NULL) {
        export = alloc_export();
        return export;
    } else { /* free resources case */
        export = self_struct;
        /* As part of create_export(), FSAL shall take
         * reference to the export if it supports pNFS.
         */
        if (export->has_pnfs_ds) {
            assert(export->refcnt == 1);
            /* export is not yet added to the export
             * manager. Hence there shall not be any
             * other thread racing here. So no need
             * to take lock. */
            export->has_pnfs_ds = false;
            pnfs_ds_remove(export->export_id, true);
        } else {
            assert(export->refcnt == 0);
            free_export(export);  /* <---------- double free memory */
        }

        return NULL;
    }
}
SergeyLysanov commented 6 years ago

Actually, issue fixed in 2.5-stable by commit 9d741775c71d1ff52e7444f3f1d791301fa58b27:

export: avoid double free if init_export_root fail at add_export

put_gsh_export() frees the export if init_export_root() fail,
and then, a double free happens in the following config code.

Change-Id: Ia94be7915ed014de1055aa9cb6c4f8089108ff0c
Signed-off-by: Kinglong Mee <mijinlong@open-fs.com>
(cherry picked from commit 484298556b2722ba59b28d41f00458a5f9749266)