Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f274e78e authored by Tom Cherry's avatar Tom Cherry
Browse files

fs_mgr/init: use unique_ptr + CLO_EXEC for setmntent()/fopen() 

We ran into an issue with an fd leaking due to missing both CLO_EXEC
and fclose() in related code, so let's make sure we're safe here too.

Test: boot
Change-Id: Ief893c936859815c78fa6d7e06cb88ad34aadbac
parent 3da42a6c

fs_mgr/fs_mgr.cpp

+11 −15
Original line number Diff line number Diff line
@@ -1235,29 +1235,25 @@ int fs_mgr_swapon_all(struct fstab *fstab) 
             * on a system (all the memory comes from the same pool) so

             * we can assume the device number is 0.

             */

            FILE *zram_fp;

            FILE *zram_mcs_fp;



            if (fstab->recs[i].max_comp_streams >= 0) {

               zram_mcs_fp = fopen(ZRAM_CONF_MCS, "r+");

                auto zram_mcs_fp = std::unique_ptr<FILE, decltype(&fclose)>{

                        fopen(ZRAM_CONF_MCS, "re"), fclose};

                if (zram_mcs_fp == NULL) {

                LERROR << "Unable to open zram conf comp device "

                       << ZRAM_CONF_MCS;

                    LERROR << "Unable to open zram conf comp device " << ZRAM_CONF_MCS;

                    ret = -1;

                    continue;

                }

              fprintf(zram_mcs_fp, "%d\n", fstab->recs[i].max_comp_streams);

              fclose(zram_mcs_fp);

                fprintf(zram_mcs_fp.get(), "%d\n", fstab->recs[i].max_comp_streams);

            }



            zram_fp = fopen(ZRAM_CONF_DEV, "r+");

            auto zram_fp =

                    std::unique_ptr<FILE, decltype(&fclose)>{fopen(ZRAM_CONF_DEV, "re+"), fclose};

            if (zram_fp == NULL) {

                LERROR << "Unable to open zram conf device " << ZRAM_CONF_DEV;

                ret = -1;

                continue;

            }

            fprintf(zram_fp, "%u\n", fstab->recs[i].zram_size);

            fclose(zram_fp);

            fprintf(zram_fp.get(), "%u\n", fstab->recs[i].zram_size);

        }



        if (fstab->recs[i].fs_mgr_flags & MF_WAIT &&

fs_mgr/fs_mgr_fstab.cpp

+2 −4
Original line number Diff line number Diff line
@@ -730,21 +730,19 @@ static std::set<std::string> extract_boot_devices(const fstab& fstab) { 


struct fstab *fs_mgr_read_fstab(const char *fstab_path)

{

    FILE *fstab_file;

    struct fstab *fstab;



    fstab_file = fopen(fstab_path, "r");

    auto fstab_file = std::unique_ptr<FILE, decltype(&fclose)>{fopen(fstab_path, "re"), fclose};

    if (!fstab_file) {

        PERROR << __FUNCTION__<< "(): cannot open file: '" << fstab_path << "'";

        return nullptr;

    }



    fstab = fs_mgr_read_fstab_file(fstab_file, !strcmp("/proc/mounts", fstab_path));

    fstab = fs_mgr_read_fstab_file(fstab_file.get(), !strcmp("/proc/mounts", fstab_path));

    if (!fstab) {

        LERROR << __FUNCTION__ << "(): failed to load fstab from : '" << fstab_path << "'";

    }



    fclose(fstab_file);

    return fstab;

}

fs_mgr/fs_mgr_verity.cpp

+22 −37
Original line number Diff line number Diff line
@@ -89,24 +89,21 @@ static RSA *load_key(const char *path) 
{

    uint8_t key_data[ANDROID_PUBKEY_ENCODED_SIZE];



    FILE* f = fopen(path, "r");

    auto f = std::unique_ptr<FILE, decltype(&fclose)>{fopen(path, "re"), fclose};

    if (!f) {

        LERROR << "Can't open " << path;

        return NULL;

        return nullptr;

    }



    if (!fread(key_data, sizeof(key_data), 1, f)) {

    if (!fread(key_data, sizeof(key_data), 1, f.get())) {

        LERROR << "Could not read key!";

        fclose(f);

        return NULL;

        return nullptr;

    }



    fclose(f);



    RSA* key = NULL;

    RSA* key = nullptr;

    if (!android_pubkey_decode(key_data, sizeof(key_data), &key)) {

        LERROR << "Could not parse key!";

        return NULL;

        return nullptr;

    }



    return key;
@@ -368,7 +365,6 @@ static int metadata_add(FILE *fp, long start, const char *tag, 
static int metadata_find(const char *fname, const char *stag,

        unsigned int slength, off64_t *offset)

{

    FILE *fp = NULL;

    char tag[METADATA_TAG_MAX_LENGTH + 1];

    int rc = -1;

    int n;
@@ -380,75 +376,64 @@ static int metadata_find(const char *fname, const char *stag, 
        return -1;

    }



    fp = fopen(fname, "r+");

    auto fp = std::unique_ptr<FILE, decltype(&fclose)>{fopen(fname, "re+"), fclose};



    if (!fp) {

        PERROR << "Failed to open " << fname;

        goto out;

        return -1;

    }



    /* check magic */

    if (fseek(fp, start, SEEK_SET) < 0 ||

        fread(&magic, sizeof(magic), 1, fp) != 1) {

    if (fseek(fp.get(), start, SEEK_SET) < 0 || fread(&magic, sizeof(magic), 1, fp.get()) != 1) {

        PERROR << "Failed to read magic from " << fname;

        goto out;

        return -1;

    }



    if (magic != METADATA_MAGIC) {

        magic = METADATA_MAGIC;



        if (fseek(fp, start, SEEK_SET) < 0 ||

            fwrite(&magic, sizeof(magic), 1, fp) != 1) {

        if (fseek(fp.get(), start, SEEK_SET) < 0 ||

            fwrite(&magic, sizeof(magic), 1, fp.get()) != 1) {

            PERROR << "Failed to write magic to " << fname;

            goto out;

            return -1;

        }



        rc = metadata_add(fp, start + sizeof(magic), stag, slength, offset);

        rc = metadata_add(fp.get(), start + sizeof(magic), stag, slength, offset);

        if (rc < 0) {

            PERROR << "Failed to add metadata to " << fname;

        }



        goto out;

        return rc;

    }



    start += sizeof(magic);



    while (1) {

        n = fscanf(fp, "%" STRINGIFY(METADATA_TAG_MAX_LENGTH) "s %u\n",

                tag, &length);

        n = fscanf(fp.get(), "%" STRINGIFY(METADATA_TAG_MAX_LENGTH) "s %u\n", tag, &length);



        if (n == 2 && strcmp(tag, METADATA_EOD)) {

            /* found a tag */

            start = ftell(fp);

            start = ftell(fp.get());



            if (!strcmp(tag, stag) && length == slength) {

                *offset = start;

                rc = 0;

                goto out;

                return 0;

            }



            start += length;



            if (fseek(fp, length, SEEK_CUR) < 0) {

            if (fseek(fp.get(), length, SEEK_CUR) < 0) {

                PERROR << "Failed to seek " << fname;

                goto out;

                return -1;

            }

        } else {

            rc = metadata_add(fp, start, stag, slength, offset);

            rc = metadata_add(fp.get(), start, stag, slength, offset);

            if (rc < 0) {

                PERROR << "Failed to write metadata to " << fname;

            }

            goto out;

        }

            return rc;

        }



out:

    if (fp) {

        fflush(fp);

        fclose(fp);

    }



    return rc;

}



static int write_verity_state(const char *fname, off64_t offset, int32_t mode)

fs_mgr/tests/fs_mgr_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ TEST(fs_mgr, fs_mgr_read_fstab_file_proc_mounts) { 
    auto fstab = fs_mgr_read_fstab("/proc/mounts");

    ASSERT_NE(fstab, nullptr);



    std::unique_ptr<std::FILE, int (*)(std::FILE*)> mounts(setmntent("/proc/mounts", "r"),

    std::unique_ptr<std::FILE, int (*)(std::FILE*)> mounts(setmntent("/proc/mounts", "re"),

                                                           endmntent);

    ASSERT_NE(mounts, nullptr);

init/reboot.cpp

+1 −1
Original line number Diff line number Diff line
@@ -162,7 +162,7 @@ static void LogShutdownTime(UmountStat stat, Timer* t) { 
 */

static bool FindPartitionsToUmount(std::vector<MountEntry>* blockDevPartitions,

                                   std::vector<MountEntry>* emulatedPartitions, bool dump) {

    std::unique_ptr<std::FILE, int (*)(std::FILE*)> fp(setmntent("/proc/mounts", "r"), endmntent);

    std::unique_ptr<std::FILE, int (*)(std::FILE*)> fp(setmntent("/proc/mounts", "re"), endmntent);

    if (fp == nullptr) {

        PLOG(ERROR) << "Failed to open /proc/mounts";

        return false;