From cfd7cd2099a1c88211b782fdeeae59350456cdde Mon Sep 17 00:00:00 2001 From: ajs124 Date: Wed, 9 Nov 2016 22:17:33 +0700 Subject: [PATCH 01/12] fix Makefile --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index e143a12..d950036 100644 --- a/Makefile +++ b/Makefile @@ -12,16 +12,16 @@ generator: $(GENERATOR_OUT) shutdown: $(SHUTDOWN_OUT) -$(MOUNT_OUT): src/mount.initrd_zfs.c src/zfs-util.c src/zfs-util.h +$(MOUNT_OUT): src/mount.initrd_zfs.c src/zfs-util.c $(CC) $(CCFLAGS) $(LDFLAGS) -o $@ $^ -$(GENERATOR_OUT): src/zfs-generator.c src/cmdline.c src/cmdline.h +$(GENERATOR_OUT): src/zfs-generator.c src/cmdline.c $(CC) $(CCFLAGS) $(LDFLAGS) -o $@ $^ $(SHUTDOWN_OUT): src/zfs-shutdown.c $(CC) $(CCFLAGS) $(LDFLAGS) -o $@ $^ clean: - $(RM) $(MOUNT_OUT) $(ZFS_GENERATOR_OUT) $(ZFS_SHUTDOWN_OUT) + $(RM) $(MOUNT_OUT) $(GENERATOR_OUT) $(SHUTDOWN_OUT) .PHONY: all mount generator shutdown clean From f65baee26290230c387c6cfd4ad1e972b0c742a6 Mon Sep 17 00:00:00 2001 From: ajs124 Date: Wed, 9 Nov 2016 22:32:23 +0700 Subject: [PATCH 02/12] misc. error message fixes --- src/cmdline.c | 4 ++-- src/zfs-generator.c | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/cmdline.c b/src/cmdline.c index 4f628cd..2006151 100644 --- a/src/cmdline.c +++ b/src/cmdline.c @@ -9,11 +9,11 @@ int getCmdline(char **cmdline) { fd = fopen("/proc/cmdline", "r"); if (fd == NULL) { - fprintf(stderr, "Can not open kernel command line\n"); + fprintf(stderr, "Cannot open kernel command line\n"); return -1; } if (getline(cmdline, &linelen, fd) < 0) { - fprintf(stderr, "Can not read kernel command line\n"); + fprintf(stderr, "Cannot read kernel command line\n"); fclose(fd); return -2; } diff --git a/src/zfs-generator.c b/src/zfs-generator.c index 7e916e3..f712483 100644 --- a/src/zfs-generator.c +++ b/src/zfs-generator.c @@ -28,7 +28,7 @@ int getRootOptions(char **options) { optionsval = malloc((strlen("zfsutil") + 1) * sizeof(char)); strcpy(optionsval, "zfsutil"); } else if (ret != 0) { - fprintf(stderr, "Unknown thing happened while reading the rootflags= paremter\n"); + fprintf(stderr, "Unknown thing happened while reading the rootflags= paramter\n"); return 2; } len = strlen(optionsval) + 1; @@ -138,12 +138,11 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa // Check if unit already exists if (access(unitpath, R_OK) != -1) { free(unitpath); - printf("Scanning unit file already exists\n"); + perror("Scanning unit file already exists or cannot be accessed\n"); return 0; } // Check if we need to ignore the cache file if (ignoreCache == 0) { - // Wir möchten die Zeile cacheLine = malloc((strlen("ConditionPathExists=!/etc/zfs/zpool.cache") + 1) * sizeof(char)); strcpy(cacheLine, "ConditionPathExists=!/etc/zfs/zpool.cache"); } else { @@ -155,7 +154,7 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa if (fp == NULL) { free(unitpath); free(cacheLine); - fprintf(stderr, "Can not write to scanning unit file\n"); + perror("Can not write to scanning unit file\n"); return 1; } fprintf(fp, "[Unit]\n\ @@ -208,7 +207,7 @@ int generateCacheUnit(char *directory, const char *targetName, const char *unitN fp = fopen(unitpath, "w"); if (fp == NULL) { free(unitpath); - fprintf(stderr, "Can not write to scanning unit file\n"); + perror("Cannot write to scanning unit file\n"); return 1; } fprintf(fp, "[Unit]\n\ @@ -250,7 +249,7 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh // Check if unit already exists if (access(unitpath, R_OK) != -1) { free(unitpath); - printf("Mounting unit file already exists\n"); + perror("Mounting unit file already exists\n"); return 0; } @@ -288,7 +287,7 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh free(unitpath); free(options); free(what); - fprintf(stderr, "Can not write to mounting unit file\n"); + perror("Can not write to mounting unit file\n"); return 1; } fprintf(fp, "[Mount]\n\ From 9957b146843e9be31601588ab284e619b76cb0c1 Mon Sep 17 00:00:00 2001 From: ajs124 Date: Thu, 10 Nov 2016 19:38:39 +0700 Subject: [PATCH 03/12] fix string length miscalculation, found by coverity --- src/mount.initrd_zfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index 5df53b0..0274052 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -157,7 +157,7 @@ int main(int argc, char *argv[]) { } snapshot = strtok(snaps, "\n"); while (snapshot != NULL) { - snapPath = malloc((strlen(snapDS) + strlen(&snapshot[strlen(dataset)] + 1) * sizeof(char))); + snapPath = malloc((strlen(snapDS) + strlen(&snapshot[strlen(dataset)]) + 1) * sizeof(char)); strcpy(snapPath, snapDS); strcat(snapPath, &(snapshot[strlen(dataset)])); at = strrchr(snapPath, '@'); From 21c9932470139cb8913e057ff9cf1122a4dee22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Thu, 10 Nov 2016 16:15:54 +0100 Subject: [PATCH 04/12] fprintf -> perror --- src/cmdline.c | 4 ++-- src/zfs-util.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmdline.c b/src/cmdline.c index 2006151..9a147c6 100644 --- a/src/cmdline.c +++ b/src/cmdline.c @@ -9,11 +9,11 @@ int getCmdline(char **cmdline) { fd = fopen("/proc/cmdline", "r"); if (fd == NULL) { - fprintf(stderr, "Cannot open kernel command line\n"); + perror("Cannot open kernel command line\n"); return -1; } if (getline(cmdline, &linelen, fd) < 0) { - fprintf(stderr, "Cannot read kernel command line\n"); + perror("Cannot read kernel command line\n"); fclose(fd); return -2; } diff --git a/src/zfs-util.c b/src/zfs-util.c index ecbcceb..e540ba5 100644 --- a/src/zfs-util.c +++ b/src/zfs-util.c @@ -39,7 +39,7 @@ int execute(char *command, char needOutput, char **output, char *param[]) { execv(command, param); exit(254); } else if (pid < 0) { - fprintf(stderr, "Can not fork\n"); + perror("Can not fork\n"); close(pip[0]); close(pip[1]); return pid; From 3d2c4f64c689b029d42f4fa100e07f70f57e1bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Thu, 10 Nov 2016 16:17:54 +0100 Subject: [PATCH 05/12] mount: Correctly initialize snap variable --- src/mount.initrd_zfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index 0274052..0256634 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -43,7 +43,7 @@ int main(int argc, char *argv[]) { char *snapshot; char *at; // For mounting - char *snap; + char *snap = NULL; char *lines = NULL; char *endLine; char *lineToken; From 0f453719d6a480022bd5ac36e4d36ec52f6174de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Thu, 10 Nov 2016 16:18:56 +0100 Subject: [PATCH 06/12] mount: Correctly NULL dataset --- src/mount.initrd_zfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index 0256634..b345ffb 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -117,6 +117,7 @@ int main(int argc, char *argv[]) { if (handleBootfs(&dataset) != 0) { fprintf(stderr, "Can not get bootfs value\n"); free(dataset); + dataset = NULL; free(mountpoint); if (options != NULL) { free(options); @@ -201,6 +202,7 @@ int main(int argc, char *argv[]) { ret = zfs_list_datasets_with_mp(dataset, &lines); if (ret != 0) { free(dataset); + dataset = NULL; free(mountpoint); if (options != NULL) { free(options); From 7786e4e4378a9d6a8792516c7a744a76ac19f044 Mon Sep 17 00:00:00 2001 From: ajs124 Date: Thu, 10 Nov 2016 22:48:37 +0700 Subject: [PATCH 07/12] man 3 free: "If ptr is NULL, no operation is performed." --- src/mount.initrd_zfs.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index b345ffb..5eeaad4 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -19,9 +19,7 @@ int handleBootfs(char **pool) { } ret = zfs_get_bootfs(rpool, pool); - if (rpool != NULL) { - free(rpool); - } + free(rpool); return ret; } @@ -70,9 +68,8 @@ int main(int argc, char *argv[]) { } strcat(newoptions, argv[i]); strcat(newoptions, ","); - if (options != NULL) { - free(options); - } + free(options); + options = newoptions; isOption = 0; continue; @@ -119,9 +116,7 @@ int main(int argc, char *argv[]) { free(dataset); dataset = NULL; free(mountpoint); - if (options != NULL) { - free(options); - } + free(options); } } @@ -194,9 +189,7 @@ int main(int argc, char *argv[]) { } } aftersnap: - if (snapDS != NULL) { - free(snapDS); - } + free(snapDS); // Mount the dataset(s) ret = zfs_list_datasets_with_mp(dataset, &lines); @@ -204,9 +197,7 @@ int main(int argc, char *argv[]) { free(dataset); dataset = NULL; free(mountpoint); - if (options != NULL) { - free(options); - } + free(options); } lineToken = strtok_r(lines, "\n", &endLine); @@ -227,10 +218,10 @@ int main(int argc, char *argv[]) { strcat(where, (where_tmp == NULL) ? mountpointToken : where_tmp); // Mount ret = zfs_mount(what, where, options); - if (where_tmp != NULL) { - free(where_tmp); - where_tmp = NULL; - } + + free(where_tmp); + where_tmp = NULL; + if (ret != 0) { fprintf(stderr, "ZFS command failed with exit code %d, bailing out\n", ret); free(where); @@ -243,10 +234,7 @@ int main(int argc, char *argv[]) { } free(lines); - free(dataset); free(mountpoint); - if (options != NULL) { - free(options); - } + free(options); } From b7b4f4e1b41d39d1ddc120f051e0014090d4992e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Tue, 22 Nov 2016 10:44:01 +0100 Subject: [PATCH 08/12] Fix potential resource leaks --- src/mount.initrd_zfs.c | 4 ++-- src/zfs-generator.c | 8 ++++++++ src/zfs-util.c | 12 +++++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index 5eeaad4..731e2fb 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -114,9 +114,9 @@ int main(int argc, char *argv[]) { if (handleBootfs(&dataset) != 0) { fprintf(stderr, "Can not get bootfs value\n"); free(dataset); - dataset = NULL; free(mountpoint); free(options); + exit(1); } } @@ -195,9 +195,9 @@ int main(int argc, char *argv[]) { ret = zfs_list_datasets_with_mp(dataset, &lines); if (ret != 0) { free(dataset); - dataset = NULL; free(mountpoint); free(options); + exit(1); } lineToken = strtok_r(lines, "\n", &endLine); diff --git a/src/zfs-generator.c b/src/zfs-generator.c index f712483..f8c9951 100644 --- a/src/zfs-generator.c +++ b/src/zfs-generator.c @@ -86,6 +86,7 @@ int getForce(char **forceParam) { *forceParam = malloc(4 * sizeof(char)); strcpy(*forceParam, " -f"); } + free(forceval); return 0; } @@ -138,6 +139,7 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa // Check if unit already exists if (access(unitpath, R_OK) != -1) { free(unitpath); + free(targetpath); perror("Scanning unit file already exists or cannot be accessed\n"); return 0; } @@ -154,6 +156,7 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa if (fp == NULL) { free(unitpath); free(cacheLine); + free(targetpath); perror("Can not write to scanning unit file\n"); return 1; } @@ -174,6 +177,7 @@ ExecStart=/usr/bin/zpool import %s -N -o cachefile=none%s\n", cacheLine, poolNam free(cacheLine); free(unitpath); + free(targetpath); return 0; } @@ -200,6 +204,7 @@ int generateCacheUnit(char *directory, const char *targetName, const char *unitN // Check if unit already exists if (access(unitpath, R_OK) != -1) { free(unitpath); + free(targetpath); printf("Caching unit file already exists\n"); return 0; } @@ -207,6 +212,7 @@ int generateCacheUnit(char *directory, const char *targetName, const char *unitN fp = fopen(unitpath, "w"); if (fp == NULL) { free(unitpath); + free(targetpath); perror("Cannot write to scanning unit file\n"); return 1; } @@ -225,6 +231,7 @@ RemainAfterExit=yes\n\ ExecStart=/usr/bin/zpool import %s -N -c /etc/zfs/zpool.cache%s\n", poolName, forceParam); fclose(fp); free(unitpath); + free(targetpath); return 0; } @@ -278,6 +285,7 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh if (getRootOptions(&options) != 0) { fprintf(stderr, "Can not get root options\n"); free(what); + free(unitpath); return 1; } diff --git a/src/zfs-util.c b/src/zfs-util.c index e540ba5..2979b68 100644 --- a/src/zfs-util.c +++ b/src/zfs-util.c @@ -33,15 +33,20 @@ int execute(char *command, char needOutput, char **output, char *param[]) { close(2); if (needOutput == 1) { close(pip[0]); - dup(pip[1]); + if (dup(pip[1]) < 0) { + perror("Can not duplicate pipe\n"); + close(pip[1]); + } } // Execute execv(command, param); exit(254); } else if (pid < 0) { perror("Can not fork\n"); - close(pip[0]); - close(pip[1]); + if (needOutput == 1) { + close(pip[0]); + close(pip[1]); + } return pid; } if (needOutput == 1) { @@ -249,6 +254,7 @@ int zfs_list_snapshots(char *dataset, char *snapshot, char **output) { } tok = strtok(NULL, "\n"); } + free(suffix); free(snaps); return 0; } From f46115ec08f57d44f570780ad38fec5b3e6108d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Tue, 22 Nov 2016 11:04:12 +0100 Subject: [PATCH 09/12] generator: Check mkdir return code --- src/zfs-generator.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/zfs-generator.c b/src/zfs-generator.c index f8c9951..5364e64 100644 --- a/src/zfs-generator.c +++ b/src/zfs-generator.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -131,7 +132,12 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa strcat(unitpath, "/"); strcat(unitpath, unitName); // Make wants directory - mkdir(targetpath, 0775); + if (mkdir(targetpath, 0775) < 0 && errno != EEXIST) { + perror("Can not create unit directory\n"); + free(targetpath); + free(unitpath); + return(1); + } // Make symlink strcat(targetpath, "/"); strcat(targetpath, unitName); @@ -196,7 +202,12 @@ int generateCacheUnit(char *directory, const char *targetName, const char *unitN strcat(unitpath, "/"); strcat(unitpath, unitName); // Make wants directory - mkdir(targetpath, 0775); + if (mkdir(targetpath, 0775) < 0 && errno != EEXIST) { + perror("Can not create unit directory\n"); + free(targetpath); + free(unitpath); + return(1); + } // Make symlink strcat(targetpath, "/"); strcat(targetpath, unitName); @@ -250,7 +261,11 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh strcat(unitpath, "/"); strcat(unitpath, targetName); // Make dropin directory - mkdir(unitpath, 0775); + if (mkdir(unitpath, 0775) < 0 && errno != EEXIST) { + perror("Can not create unit directory\n"); + free(unitpath); + return(1); + } strcat(unitpath, "/"); strcat(unitpath, unitName); // Check if unit already exists From dc9401ff8a91290900dd843ea1c1f55544f1d264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Tue, 22 Nov 2016 11:12:49 +0100 Subject: [PATCH 10/12] Correctly order perror --- src/zfs-generator.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/zfs-generator.c b/src/zfs-generator.c index 5364e64..f67570c 100644 --- a/src/zfs-generator.c +++ b/src/zfs-generator.c @@ -144,9 +144,9 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa symlink(unitName, targetpath); // Check if unit already exists if (access(unitpath, R_OK) != -1) { + perror("Scanning unit file already exists or cannot be accessed\n"); free(unitpath); free(targetpath); - perror("Scanning unit file already exists or cannot be accessed\n"); return 0; } // Check if we need to ignore the cache file @@ -160,10 +160,10 @@ int generateScanUnit(char *directory, const char *targetName, const char *unitNa // Write fp = fopen(unitpath, "w"); if (fp == NULL) { + perror("Can not write to scanning unit file\n"); free(unitpath); free(cacheLine); free(targetpath); - perror("Can not write to scanning unit file\n"); return 1; } fprintf(fp, "[Unit]\n\ @@ -222,9 +222,9 @@ int generateCacheUnit(char *directory, const char *targetName, const char *unitN // Write fp = fopen(unitpath, "w"); if (fp == NULL) { + perror("Cannot write to scanning unit file\n"); free(unitpath); free(targetpath); - perror("Cannot write to scanning unit file\n"); return 1; } fprintf(fp, "[Unit]\n\ @@ -270,8 +270,8 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh strcat(unitpath, unitName); // Check if unit already exists if (access(unitpath, R_OK) != -1) { - free(unitpath); perror("Mounting unit file already exists\n"); + free(unitpath); return 0; } @@ -307,10 +307,10 @@ int generateSysrootUnit(char *directory, int bootfs, char *dataset, char *snapsh // Write fp = fopen(unitpath, "w"); if (fp == NULL) { + perror("Can not write to mounting unit file\n"); free(unitpath); free(options); free(what); - perror("Can not write to mounting unit file\n"); return 1; } fprintf(fp, "[Mount]\n\ From 4411dfd63f48982f08130606110cb1e8d2f5ecd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Tue, 22 Nov 2016 11:15:59 +0100 Subject: [PATCH 11/12] Check if all parameters are supplied --- src/mount.initrd_zfs.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mount.initrd_zfs.c b/src/mount.initrd_zfs.c index 731e2fb..091c492 100644 --- a/src/mount.initrd_zfs.c +++ b/src/mount.initrd_zfs.c @@ -97,6 +97,11 @@ int main(int argc, char *argv[]) { fprintf(stderr, "Call like: %s [-o option[,...]]\n", argv[0]); exit(1); } + // Check if all parameters are supplied + if (dataset == NULL || mountpoint == NULL) { + fprintf(stderr, "Call like: %s [-o option[,...]]\n", argv[0]); + exit(1); + } // Get rid of the trailing comma if (options != NULL) { newoptions = malloc(sizeof(options) - sizeof(char)); From 34494b0c09683e748f6bf868207ea10d70943a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Tue, 22 Nov 2016 11:30:55 +0100 Subject: [PATCH 12/12] util: Close output if exec fails --- src/zfs-util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/zfs-util.c b/src/zfs-util.c index 2979b68..e0c0a72 100644 --- a/src/zfs-util.c +++ b/src/zfs-util.c @@ -21,6 +21,7 @@ int execute(char *command, char needOutput, char **output, char *param[]) { char *linebuffer; size_t size; int nRead; + int fd; // Execute if (needOutput == 1) { @@ -33,13 +34,15 @@ int execute(char *command, char needOutput, char **output, char *param[]) { close(2); if (needOutput == 1) { close(pip[0]); - if (dup(pip[1]) < 0) { + fd = dup(pip[1]); + if (fd < 0) { perror("Can not duplicate pipe\n"); close(pip[1]); } } // Execute execv(command, param); + close(fd); exit(254); } else if (pid < 0) { perror("Can not fork\n");