From ba31a5018f99864c22dd4e0f10712456c7abc934 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 22 Mar 2024 18:26:55 +0800 Subject: [PATCH 1/2] core/swap: fix memory management in swap_setup_unit Follow-up for e9fa1bf704ad2f0a7e257e29889315118b0df459 --- src/core/swap.c | 50 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/src/core/swap.c b/src/core/swap.c index e2019587ff..afd067f0e0 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -374,11 +374,10 @@ static int swap_setup_unit( int priority, bool set_flags) { + _cleanup_(unit_freep) Unit *new = NULL; _cleanup_free_ char *e = NULL; - bool new; Unit *u; Swap *s; - SwapParameters *p; int r; assert(m); @@ -395,43 +394,34 @@ static int swap_setup_unit( if (s->from_proc_swaps && !path_equal(s->parameters_proc_swaps.what, what_proc_swaps)) - return log_error_errno(SYNTHETIC_ERRNO(EEXIST), - "Swap %s appeared twice with different device paths %s and %s, refusing.", - e, s->parameters_proc_swaps.what, what_proc_swaps); - - new = false; + return log_unit_error_errno(u, SYNTHETIC_ERRNO(EEXIST), + "Swap appeared twice with different device paths %s and %s, refusing.", + s->parameters_proc_swaps.what, what_proc_swaps); } else { - new = true; - - r = unit_new_for_name(m, sizeof(Swap), e, &u); - if (r < 0) { - log_unit_warning_errno(u, r, "Failed to load swap unit: %m"); - goto fail; - } + r = unit_new_for_name(m, sizeof(Swap), e, &new); + if (r < 0) + return log_warning_errno(r, "Failed to load swap unit '%s': %m", e); + u = new; s = ASSERT_PTR(SWAP(u)); s->what = strdup(what); - if (s->what) { - r = log_oom(); - goto fail; - } + if (!s->what) + return log_oom(); unit_add_to_load_queue(u); } - p = &s->parameters_proc_swaps; + SwapParameters *p = &s->parameters_proc_swaps; - if (!s->parameters_proc_swaps.what) { + if (!p->what) { p->what = strdup(what_proc_swaps); - if (!p->what) { - r = log_oom(); - goto fail; - } + if (!p->what) + return log_oom(); } - /* The unit is definitely around now, mark it as loaded if it was previously referenced but could not be - * loaded. After all we can load it now, from the data in /proc/swaps. */ + /* The unit is definitely around now, mark it as loaded if it was previously referenced but + * could not be loaded. After all we can load it now, from the data in /proc/swaps. */ if (IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_ERROR)) { u->load_state = UNIT_LOADED; u->load_error = 0; @@ -439,7 +429,7 @@ static int swap_setup_unit( if (set_flags) { s->is_active = true; - s->just_activated = !SWAP(u)->from_proc_swaps; + s->just_activated = !s->from_proc_swaps; } s->from_proc_swaps = true; @@ -449,12 +439,6 @@ static int swap_setup_unit( unit_add_to_dbus_queue(u); return 0; - -fail: - if (!new) - unit_free(u); - - return r; } static void swap_process_new(Manager *m, const char *device, int prio, bool set_flags) { From f1dfc20a4aeb6cb7d8867e1465cd025aa8ae75ab Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 22 Mar 2024 18:36:01 +0800 Subject: [PATCH 2/2] core/mount: use ASSERT_PTR in mount_setup_new_unit --- src/core/mount.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 0d346c985f..f574e28ec4 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1689,6 +1689,7 @@ static int mount_setup_new_unit( Unit **ret) { _cleanup_(unit_freep) Unit *u = NULL; + Mount *mnt; int r; assert(m); @@ -1700,24 +1701,26 @@ static int mount_setup_new_unit( if (r < 0) return r; + mnt = ASSERT_PTR(MOUNT(u)); + r = free_and_strdup(&u->source_path, "/proc/self/mountinfo"); if (r < 0) return r; - r = free_and_strdup(&MOUNT(u)->where, where); + r = free_and_strdup(&mnt->where, where); if (r < 0) return r; - r = update_parameters_proc_self_mountinfo(MOUNT(u), what, options, fstype); + r = update_parameters_proc_self_mountinfo(mnt, what, options, fstype); if (r < 0) return r; /* This unit was generated because /proc/self/mountinfo reported it. Remember this, so that by the * time we load the unit file for it (and thus add in extra deps right after) we know what source to * attributes the deps to. */ - MOUNT(u)->from_proc_self_mountinfo = true; + mnt->from_proc_self_mountinfo = true; - r = mount_add_non_exec_dependencies(MOUNT(u)); + r = mount_add_non_exec_dependencies(mnt); if (r < 0) return r; @@ -1742,6 +1745,7 @@ static int mount_setup_existing_unit( int r; assert(u); + assert(where); assert(ret_flags); if (!m->where) {