fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-02 11:10:16 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

audit: don't use GValue for tracking values in AuditField struct

Browse source 
When using a GValue, we really should call g_value_unset(). Otherwise
it is a code smell, even if we technically only created GValue with
static strings and integers.

But changing that is not easy, because the AuditField structs are
allocated on the stack, and in different functions. So we cannot just
pass a GDestroyNotify to GPtrArray to cleanup all those fields, because
by then they will be out of scope.

The proper solution would be to heap allocate the AuditField struct, add
them to the GPtrArray, and free them with the free function. But that
seams really unnecessary overhead, for something that is correct in
practice. Let's accept the fact that by the time the fields array gets
destroyed, it contains dangling pointers.

If we already embrace the dangling pointers and that stuff is allocated
on the stack and that we don't need to free, also get rid of GValue
and use our plain NMValueType and NMValueTypUnion. GValue really doesn't
give us much here. And it only makes us wonder: is it OK to not call
g_value_unset()? With the plain tracking of the values, we know that
it is OK.
This commit is contained in:
Thomas Haller 2021-04-09 14:08:00 +02:00
parent ba2bb8d741
commit c82b7c94c0
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
1 changed files with 59 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

95
src/core/nm-audit-manager.c
View file

@ -11,15 +11,18 @@
#include <libaudit.h>
#endif
#include "libnm-glib-aux/nm-str-buf.h"
#define NM_VALUE_TYPE_DEFINE_FUNCTIONS
#include "libnm-core-aux-intern/nm-auth-subject.h"
#include "libnm-glib-aux/nm-str-buf.h"
#include "libnm-glib-aux/nm-value-type.h"
#include "nm-config.h"
#include "nm-dbus-manager.h"
#include "settings/nm-settings-connection.h"
/*****************************************************************************/
typedef enum {
typedef enum _nm_packed {
BACKEND_LOG = (1 << 0),
BACKEND_AUDITD = (1 << 1),
_BACKEND_LAST,
@ -27,10 +30,11 @@ typedef enum {
} AuditBackend;
typedef struct {
const char * name;
GValue value;
gboolean need_encoding;
AuditBackend backends;
const char * name;
AuditBackend backends;
bool need_encoding;
NMValueType value_type;
NMValueTypUnion value;
} AuditField;
/*****************************************************************************/
@ -86,20 +90,24 @@ _audit_field_init_string(AuditField * field,
gboolean need_encoding,
AuditBackend backends)
{
field->name = name;
field->need_encoding = need_encoding;
field->backends = backends;
g_value_init(&field->value, G_TYPE_STRING);
g_value_set_static_string(&field->value, str);
*field = (AuditField){
.name = name,
.need_encoding = need_encoding,
.backends = backends,
.value_type = NM_VALUE_TYPE_STRING,
.value.v_string = str,
};
}
static void
_audit_field_init_uint(AuditField *field, const char *name, uint val, AuditBackend backends)
_audit_field_init_uint64(AuditField *field, const char *name, guint64 val, AuditBackend backends)
{
field->name = name;
field->backends = backends;
g_value_init(&field->value, G_TYPE_UINT);
g_value_set_uint(&field->value, val);
*field = (AuditField){
.name = name,
.backends = backends,
.value_type = NM_VALUE_TYPE_UINT64,
.value.v_uint64 = val,
};
}
static char *
@ -109,15 +117,15 @@ build_message(GPtrArray *fields, AuditBackend backend)
guint i;
for (i = 0; i < fields->len; i++) {
AuditField *field = fields->pdata[i];
const AuditField *field = fields->pdata[i];
if (!NM_FLAGS_ANY(field->backends, backend))
continue;
nm_str_buf_append_required_delimiter(&strbuf, ' ');
if (G_VALUE_HOLDS_STRING(&field->value)) {
const char *str = g_value_get_string(&field->value);
if (field->value_type == NM_VALUE_TYPE_STRING) {
const char *str = field->value.v_string;
#if HAVE_LIBAUDIT
if (backend == BACKEND_AUDITD) {
@ -131,15 +139,22 @@ build_message(GPtrArray *fields, AuditBackend backend)
continue;
}
#endif /* HAVE_LIBAUDIT */
nm_str_buf_append_printf(&strbuf, "%s=\"%s\"", field->name, str);
} else if (G_VALUE_HOLDS_UINT(&field->value)) {
continue;
}
if (field->value_type == NM_VALUE_TYPE_UINT64) {
nm_str_buf_append_printf(&strbuf,
"%s=%u",
"%s=%" G_GUINT64_FORMAT,
field->name,
g_value_get_uint(&field->value));
} else
g_assert_not_reached();
field->value.v_uint64);
continue;
}
g_return_val_if_reached(NULL);
}
return nm_str_buf_finalize(&strbuf, NULL);
}
@ -197,9 +212,13 @@ _audit_log_helper(NMAuditManager *self,
gpointer subject_context,
const char * reason)
{
AuditField op_field = {}, pid_field = {}, uid_field = {};
AuditField result_field = {}, reason_field = {};
gulong pid, uid;
AuditField op_field;
AuditField pid_field;
AuditField uid_field;
AuditField result_field;
AuditField reason_field;
gulong pid;
gulong uid;
NMAuthSubject * subject = NULL;
gs_unref_object NMAuthSubject *subject_free = NULL;
@ -220,11 +239,11 @@ _audit_log_helper(NMAuditManager *self,
pid = nm_auth_subject_get_unix_process_pid(subject);
uid = nm_auth_subject_get_unix_process_uid(subject);
if (pid != G_MAXULONG) {
_audit_field_init_uint(&pid_field, "pid", pid, BACKEND_ALL);
_audit_field_init_uint64(&pid_field, "pid", pid, BACKEND_ALL);
g_ptr_array_add(fields, &pid_field);
}
if (uid != G_MAXULONG) {
_audit_field_init_uint(&uid_field, "uid", uid, BACKEND_ALL);
_audit_field_init_uint64(&uid_field, "uid", uid, BACKEND_ALL);
g_ptr_array_add(fields, &uid_field);
}
}
@ -269,8 +288,10 @@ _nm_audit_manager_log_connection_op(NMAuditManager * self,
gpointer subject_context,
const char * reason)
{
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField uuid_field = {}, name_field = {}, args_field = {};
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField uuid_field;
AuditField name_field;
AuditField args_field;
g_return_if_fail(op);
@ -311,8 +332,8 @@ _nm_audit_manager_log_generic_op(NMAuditManager *self,
gpointer subject_context,
const char * reason)
{
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField arg_field = {};
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField arg_field;
g_return_if_fail(op);
g_return_if_fail(arg);
@ -337,8 +358,10 @@ _nm_audit_manager_log_device_op(NMAuditManager *self,
gpointer subject_context,
const char * reason)
{
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField interface_field = {}, ifindex_field = {}, args_field = {};
gs_unref_ptrarray GPtrArray *fields = NULL;
AuditField interface_field;
AuditField ifindex_field;
AuditField args_field;
int ifindex;
g_return_if_fail(op);
@ -355,7 +378,7 @@ _nm_audit_manager_log_device_op(NMAuditManager *self,
ifindex = nm_device_get_ip_ifindex(device);
if (ifindex > 0) {
_audit_field_init_uint(&ifindex_field, "ifindex", ifindex, BACKEND_ALL);
_audit_field_init_uint64(&ifindex_field, "ifindex", ifindex, BACKEND_ALL);
g_ptr_array_add(fields, &ifindex_field);
}