cmd: add functions to load/save fstab-like files #2775

Merged
merged 6 commits into from Feb 14, 2017
View
@@ -18,11 +18,12 @@ po/snappy.pot
.dirstamp
cmd/decode-mount-opts/decode-mount-opts
cmd/libsnap-confine-private/unit-tests
-cmd/snap-update-ns/snap-update-ns
cmd/snap-confine/snap-confine
cmd/snap-confine/snap-confine-unit-tests
cmd/snap-confine/snap-confine.apparmor
cmd/snap-discard-ns/snap-discard-ns
+cmd/snap-update-ns/snap-update-ns
+cmd/snap-update-ns/unit-tests
cmd/system-shutdown/system-shutdown
cmd/system-shutdown/system-shutdown-unit-tests
View
@@ -35,10 +35,11 @@ endif
.PHONY: check-unit-tests
if WITH_UNIT_TESTS
-check-unit-tests: snap-confine/snap-confine-unit-tests system-shutdown/system-shutdown-unit-tests libsnap-confine-private/unit-tests
+check-unit-tests: snap-confine/snap-confine-unit-tests system-shutdown/system-shutdown-unit-tests libsnap-confine-private/unit-tests snap-update-ns/unit-tests
+ $(HAVE_VALGRIND) ./libsnap-confine-private/unit-tests
$(HAVE_VALGRIND) ./snap-confine/snap-confine-unit-tests
+ $(HAVE_VALGRIND) ./snap-update-ns/unit-tests
$(HAVE_VALGRIND) ./system-shutdown/system-shutdown-unit-tests
- $(HAVE_VALGRIND) ./libsnap-confine-private/unit-tests
else
check-unit-tests:
echo "unit tests are disabled (rebuild with --enable-unit-tests)"
@@ -339,6 +340,22 @@ EXTRA_DIST += snap-update-ns/snap-update-ns.rst
snap_update_ns_snap_update_ns_LDADD = libsnap-confine-private.a
+snap_update_ns_snap_update_ns_SOURCES = \
+ snap-update-ns/snap-update-ns.c \
+ snap-update-ns/mount-entry.h \
+ snap-update-ns/mount-entry.c
+
snap-update-ns/%.5: snap-update-ns/%.rst
mkdir -p snap-update-ns
$(HAVE_RST2MAN) $^ > $@
+
+if WITH_UNIT_TESTS
+noinst_PROGRAMS += snap-update-ns/unit-tests
+snap_update_ns_unit_tests_SOURCES = \
+ libsnap-confine-private/unit-tests-main.c \
+ libsnap-confine-private/unit-tests.c \
+ snap-update-ns/mount-entry-test.c
+snap_update_ns_unit_tests_LDADD = libsnap-confine-private.a
+snap_update_ns_unit_tests_CFLAGS = $(GLIB_CFLAGS)
+snap_update_ns_unit_tests_LDADD += $(GLIB_LIBS)
+endif
@@ -0,0 +1,200 @@
+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "mount-entry.h"
+#include "mount-entry.c"
+
+#include <stdarg.h>
+
+#include <glib.h>
+
+static const char *test_entry_str_1 = "fsname-1 dir-1 type-1 opts-1 1 2";
+static const char *test_entry_str_2 = "fsname-2 dir-2 type-2 opts-2 3 4";
+
+static const struct sc_mount_entry test_entry_1 = {
+ .entry = {
+ .mnt_fsname = "fsname-1",
+ .mnt_dir = "dir-1",
+ .mnt_type = "type-1",
+ .mnt_opts = "opts-1",
+ .mnt_freq = 1,
+ .mnt_passno = 2,
+ }
+};
+
+static void test_looks_like_test_entry_1(const struct sc_mount_entry *entry)
+{
+ g_assert_cmpstr(entry->entry.mnt_fsname, ==, "fsname-1");
+ g_assert_cmpstr(entry->entry.mnt_dir, ==, "dir-1");
+ g_assert_cmpstr(entry->entry.mnt_type, ==, "type-1");
+ g_assert_cmpstr(entry->entry.mnt_opts, ==, "opts-1");
+ g_assert_cmpint(entry->entry.mnt_freq, ==, 1);
+ g_assert_cmpint(entry->entry.mnt_passno, ==, 2);
+}
+
+static const struct sc_mount_entry test_entry_2 = {
+ .entry = {
+ .mnt_fsname = "fsname-2",
+ .mnt_dir = "dir-2",
+ .mnt_type = "type-2",
+ .mnt_opts = "opts-2",
+ .mnt_freq = 3,
+ .mnt_passno = 4,
+ }
+};
+
+static void test_looks_like_test_entry_2(const struct sc_mount_entry *entry)
+{
+ g_assert_cmpstr(entry->entry.mnt_fsname, ==, "fsname-2");
+ g_assert_cmpstr(entry->entry.mnt_dir, ==, "dir-2");
+ g_assert_cmpstr(entry->entry.mnt_type, ==, "type-2");
+ g_assert_cmpstr(entry->entry.mnt_opts, ==, "opts-2");
+ g_assert_cmpint(entry->entry.mnt_freq, ==, 3);
+ g_assert_cmpint(entry->entry.mnt_passno, ==, 4);
+}
+
+static const struct mntent test_mnt_1 = {
+ .mnt_fsname = "fsname-1",
+ .mnt_dir = "dir-1",
+ .mnt_type = "type-1",
+ .mnt_opts = "opts-1",
+ .mnt_freq = 1,
+ .mnt_passno = 2,
+};
+
+static const struct mntent test_mnt_2 = {
+ .mnt_fsname = "fsname-2",
+ .mnt_dir = "dir-2",
+ .mnt_type = "type-2",
+ .mnt_opts = "opts-2",
+ .mnt_freq = 3,
+ .mnt_passno = 4,
+};
+
+static void test_write_lines(const char *name, ...) __attribute__ ((sentinel));
+
+static void test_remove_file(const char *name)
+{
+ remove(name);
+}
+
+static void test_write_lines(const char *name, ...)
+{
+ FILE *f = NULL;
+ f = fopen(name, "wt");
+ va_list ap;
+ va_start(ap, name);
+ const char *line;
+ while ((line = va_arg(ap, const char *)) != NULL) {
+ fprintf(f, "%s\n", line);
+ }
+ va_end(ap);
+ fclose(f);
+
+ // Cast-away the const qualifier. This just calls unlink and we don't
+ // modify the name in any way. This way the signature is compatible with
+ // that of GDestroyNotify.
+ g_test_queue_destroy((GDestroyNotify) test_remove_file, (char *)name);
+}
+
+static void test_sc_load_mount_profile()
+{
+ struct sc_mount_entry *fstab
+ __attribute__ ((cleanup(sc_cleanup_mount_entry_list))) = NULL;
+ struct sc_mount_entry *entry;
+ test_write_lines("test.fstab", test_entry_str_1, test_entry_str_2,
+ NULL);
+ fstab = sc_load_mount_profile("test.fstab");
+ g_assert_nonnull(fstab);
+
+ entry = fstab;
+ test_looks_like_test_entry_1(entry);
+ g_assert_nonnull(entry->next);
+
+ entry = entry->next;
+ test_looks_like_test_entry_2(entry);
+ g_assert_null(entry->next);
+}
+
+static void test_sc_load_mount_profile__no_such_file()
+{
+ struct sc_mount_entry *fstab
+ __attribute__ ((cleanup(sc_cleanup_mount_entry_list))) = NULL;
+ fstab = sc_load_mount_profile("test.does-not-exist.fstab");
+ g_assert_null(fstab);
+}
+
+static void test_sc_save_mount_profile()
+{
+ struct sc_mount_entry entry_1 = test_entry_1;
+ struct sc_mount_entry entry_2 = test_entry_2;
+ entry_1.next = &entry_2;
+ entry_2.next = NULL;
+
+ // We can save the profile defined above.
+ sc_save_mount_profile(&entry_1, "test.fstab");
+
+ // Cast-away the const qualifier. This just calls unlink and we don't
+ // modify the name in any way. This way the signature is compatible with
+ // that of GDestroyNotify.
+ g_test_queue_destroy((GDestroyNotify) test_remove_file,
+ (char *)"test.fstab");
+
+ // After reading the generated file it looks as expected.
+ FILE *f = fopen("test.fstab", "rt");
+ g_assert_nonnull(f);
+ char *line = NULL;
+ size_t n = 0;
+ ssize_t num_read;
+
+ num_read = getline(&line, &n, f);
+ g_assert_cmpint(num_read, >, -0);
+ g_assert_cmpstr(line, ==, "fsname-1 dir-1 type-1 opts-1 1 2\n");
+
+ num_read = getline(&line, &n, f);
+ g_assert_cmpint(num_read, >, -0);
+ g_assert_cmpstr(line, ==, "fsname-2 dir-2 type-2 opts-2 3 4\n");
+
+ num_read = getline(&line, &n, f);
+ g_assert_cmpint(num_read, ==, -1);
+
+ free(line);
+ fclose(f);
+}
+
+static void test_sc_clone_mount_entry_from_mntent()
+{
+ struct sc_mount_entry *entry =
+ sc_clone_mount_entry_from_mntent(&test_mnt_1);
+ test_looks_like_test_entry_1(entry);
+ g_assert_null(entry->next);
+
+ struct sc_mount_entry *next = sc_get_next_and_free_mount_entry(entry);
+ g_assert_null(next);
+}
+
+static void __attribute__ ((constructor)) init()
+{
+ g_test_add_func("/mount-entry/sc_load_mount_profile",
+ test_sc_load_mount_profile);
+ g_test_add_func("/mount-entry/sc_load_mount_profile/no_such_file",
+ test_sc_load_mount_profile__no_such_file);
+ g_test_add_func("/mount-entry/sc_save_mount_profile",
+ test_sc_save_mount_profile);
+ g_test_add_func("/mount-entry/test_sc_clone_mount_entry_from_mntent",
+ test_sc_clone_mount_entry_from_mntent);
+}
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "snap-update-ns/mount-entry.h"
+
+#include <errno.h>
+#include <mntent.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "../libsnap-confine-private/utils.h"
+#include "../libsnap-confine-private/string-utils.h"
+#include "../libsnap-confine-private/cleanup-funcs.h"
+
+/**
+ * Copy struct mntent into a freshly-allocated struct sc_mount_entry.
+ *
+ * The next pointer is initialized to NULL, it should be managed by the caller.
+ * If anything goes wrong the routine die()s.
+ **/
+static struct sc_mount_entry *sc_clone_mount_entry_from_mntent(const struct
+ mntent *entry)
+{
+ struct sc_mount_entry *result;
+ result = calloc(1, sizeof *result);
+ if (result == NULL) {
+ die("cannot allocate memory");
+ }
+ result->entry.mnt_fsname = strdup(entry->mnt_fsname ? : "");
+ if (result->entry.mnt_fsname == NULL) {
+ die("cannot copy string");
+ }
+ result->entry.mnt_dir = strdup(entry->mnt_dir ? : "");
+ if (result->entry.mnt_dir == NULL) {
+ die("cannot copy string");
+ }
+ result->entry.mnt_type = strdup(entry->mnt_type ? : "");
+ if (result->entry.mnt_type == NULL) {
+ die("cannot copy string");
+ }
+ result->entry.mnt_opts = strdup(entry->mnt_opts ? : "");
+ if (result->entry.mnt_opts == NULL) {
+ die("cannot copy string");
+ }
+ result->entry.mnt_freq = entry->mnt_freq;
+ result->entry.mnt_passno = entry->mnt_passno;
+ return result;
+}
+
+static struct sc_mount_entry *sc_get_next_and_free_mount_entry(struct
+ sc_mount_entry
+ *entry)
+{
+ struct sc_mount_entry *next = entry->next;
+ free(entry->entry.mnt_fsname);
+ free(entry->entry.mnt_dir);
+ free(entry->entry.mnt_type);
+ free(entry->entry.mnt_opts);
+ memset(entry, 0, sizeof *entry);
+ free(entry);
+ return next;
+}
+
+void sc_free_mount_entry_list(struct sc_mount_entry *entry)
+{
+ while (entry != NULL) {
+ entry = sc_get_next_and_free_mount_entry(entry);
+ }
+}
+
+void sc_cleanup_mount_entry_list(struct sc_mount_entry **entryp)
+{
+ if (entryp != NULL) {
+ sc_free_mount_entry_list(*entryp);
+ *entryp = NULL;
+ }
+}
+
+struct sc_mount_entry *sc_load_mount_profile(const char *pathname)
+{
+ FILE *f __attribute__ ((cleanup(sc_cleanup_endmntent))) = NULL;
+
+ f = setmntent(pathname, "rt");
@niemeyer

niemeyer Feb 13, 2017

Contributor

What an awkward function name.

@zyga

zyga Feb 13, 2017

Contributor

Indeed, but that's libc :-(

+ if (f == NULL) {
+ // NOTE: it is fine if the profile doesn't exist.
+ // It is equivalent to having no entries.
+ if (errno != ENOENT) {
+ die("cannot open mount profile %s for reading",
+ pathname);
+ }
+ return NULL;
@tyhicks

tyhicks Feb 13, 2017

Contributor

As mentioned above, we'd see a segfault here due to sc_cleanup_mount_entry_list() not handling a NULL pointer.

@tyhicks

tyhicks Feb 13, 2017

Contributor

How about adding a test for sc_load_mount_profile() where you pass it a path to a file that does not exist in order to test this case and keep it from regressing.

@zyga

zyga Feb 13, 2017

Contributor

+1, will do

@zyga

zyga Feb 14, 2017

Contributor

Done

+ }
+ // Loop over the entries in the file and copy them to a singly-linked list.
+ struct sc_mount_entry *entry, *first = NULL, *prev = NULL;
+ struct mntent *mntent_entry;
+ while (((mntent_entry = getmntent(f)) != NULL)) {
+ entry = sc_clone_mount_entry_from_mntent(mntent_entry);
+ if (prev != NULL) {
+ prev->next = entry;
+ }
+ if (first == NULL) {
+ first = entry;
+ }
+ prev = entry;
+ }
+ return first;
+}
+
+void sc_save_mount_profile(const struct sc_mount_entry *first,
+ const char *pathname)
+{
+ FILE *f __attribute__ ((cleanup(sc_cleanup_endmntent))) = NULL;
+
+ f = setmntent(pathname, "wt");
+ if (f == NULL) {
+ die("cannot open mount profile %s for writing", pathname);
+ }
+
+ const struct sc_mount_entry *entry;
+ for (entry = first; entry != NULL; entry = entry->next) {
+ if (addmntent(f, &entry->entry) != 0) {
+ die("cannot add mount entry to %s", pathname);
+ }
+ }
+}
Oops, something went wrong.