error: preserve errno in crun_error_release

[infomaniac777]

Summary

crun_error_release() and crun_error_write_warning_and_release() free internal error heap buffers (ptr->msg and ptr). POSIX explicitly permits free() to modify errno. In scenarios where crun_error_release(err) is invoked to clean up previous error state right after a failed system call, any modification to errno during deallocation can clobber the error code before callers can record it with crun_make_error(err, errno, ...).

This patch preserves errno upon entry using const int saved_errno = errno; and restores it before returning.

Changes

• Save const int saved_errno = errno; at the start of crun_error_release() and crun_error_write_warning_and_release(), and restore errno = saved_errno; before exit.
• Include <errno.h> in src/libcrun/error.c.
• Add unit tests in tests/tests_libcrun_errors.c verifying that crun_error_release() and crun_error_write_warning_and_release() preserve errno.

Fixes: #2153

[kolyshkin]

@eriksjolund PTAL

[eriksjolund]

@giuseppe If I understand your comment correctly, we would also need to fix

crun/src/libcrun/cgroup.c

Lines 164 to 165 in 26ea57f

	crun_error_release (err);
	return crun_make_error (err, errno, "error when using statfs on `%s`", CGROUP_ROOT "/freezer");

before closing #2153. In other words, we add a safety belt but we also fix users of crun_error_release() so that the safety belt could (theoretically) be removed in the future.

[giuseppe]

@giuseppe If I understand your comment correctly, we would also need to fix

crun/src/libcrun/cgroup.c

Lines 164 to 165 in 26ea57f

	crun_error_release (err);
	return crun_make_error (err, errno, "error when using statfs on `%s`", CGROUP_ROOT "/freezer");

before closing #2153. In other words, we add a safety belt but we also fix users of crun_error_release() so that the safety belt could (theoretically) be removed in the future.

I think it is fine to keep the safety belt, it is not really expensive, so that we don't risk regressions with new callers.

@infomaniac777 Please use your real name and email both for the commit author and the Signed-off-by line in the commit messsage

[infomaniac777]

Updated the commit with real name and email for the DCO sign-off, and addressed @eriksjolund's review suggestions in the unit tests (checking open_memstream for NULL, saving errno prior to fclose, and consolidating assertions).

[kolyshkin]

I would drop the tests entirely as it seems they don't really test anything and are not red without the fix. Also the part that checks crun_error_release (NULL) looks identical to test_crun_error_release_null.

To make the test fail before the fix, we either need to have our own customized free/fprintf (using LD_PRELOAD os something like this), and it's just not worth it.

[kolyshkin]

nit: this is not needed as utils.h already includes errno.h.

[giuseppe]

@infomaniac777 Please fix this one and we can merge

[eriksjolund]

Thanks, my remarks have all been addressed.