Skip to content

Commit cf02245

Browse files
committed
seccomp: refactor error handling
Fix error leak. Fix incorrect assumption about err value. Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
1 parent e954750 commit cf02245

File tree

1 file changed

+92
-49
lines changed

1 file changed

+92
-49
lines changed

src/libcrun/seccomp.c

Lines changed: 92 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -102,36 +102,54 @@ syscall_seccomp (unsigned int operation, unsigned int flags, void *args)
102102
return (int) syscall (__NR_seccomp, operation, flags, args);
103103
}
104104

105-
static unsigned long
106-
get_seccomp_operator (const char *name, libcrun_error_t *err)
107-
{
108105
#ifdef HAVE_SECCOMP
106+
static int
107+
get_seccomp_operator (const char *name, unsigned long *op, libcrun_error_t *err)
108+
{
109109
if (strcmp (name, "SCMP_CMP_NE") == 0)
110-
return SCMP_CMP_NE;
111-
if (strcmp (name, "SCMP_CMP_LT") == 0)
112-
return SCMP_CMP_LT;
113-
if (strcmp (name, "SCMP_CMP_LE") == 0)
114-
return SCMP_CMP_LE;
115-
if (strcmp (name, "SCMP_CMP_EQ") == 0)
116-
return SCMP_CMP_EQ;
117-
if (strcmp (name, "SCMP_CMP_GE") == 0)
118-
return SCMP_CMP_GE;
119-
if (strcmp (name, "SCMP_CMP_GT") == 0)
120-
return SCMP_CMP_GT;
121-
if (strcmp (name, "SCMP_CMP_MASKED_EQ") == 0)
122-
return SCMP_CMP_MASKED_EQ;
123-
124-
crun_make_error (err, 0, "seccomp get operator `%s`", name);
125-
return 0;
126-
#else
127-
return 0;
128-
#endif
110+
{
111+
*op = SCMP_CMP_NE;
112+
return 0;
113+
}
114+
else if (strcmp (name, "SCMP_CMP_LT") == 0)
115+
{
116+
*op = SCMP_CMP_LT;
117+
return 0;
118+
}
119+
else if (strcmp (name, "SCMP_CMP_LE") == 0)
120+
{
121+
*op = SCMP_CMP_LE;
122+
return 0;
123+
}
124+
else if (strcmp (name, "SCMP_CMP_EQ") == 0)
125+
{
126+
*op = SCMP_CMP_EQ;
127+
return 0;
128+
}
129+
else if (strcmp (name, "SCMP_CMP_GE") == 0)
130+
{
131+
*op = SCMP_CMP_GE;
132+
return 0;
133+
}
134+
else if (strcmp (name, "SCMP_CMP_GT") == 0)
135+
{
136+
*op = SCMP_CMP_GT;
137+
return 0;
138+
}
139+
else if (strcmp (name, "SCMP_CMP_MASKED_EQ") == 0)
140+
{
141+
*op = SCMP_CMP_MASKED_EQ;
142+
return 0;
143+
}
144+
145+
return crun_make_error (err, 0, "seccomp get operator `%s`", name);
129146
}
147+
#endif
130148

131-
static unsigned long long
132-
get_seccomp_action (const char *name, int errno_ret, libcrun_error_t *err)
133-
{
134149
#ifdef HAVE_SECCOMP
150+
static int
151+
get_seccomp_action (const char *name, int errno_ret, uint32_t *action, libcrun_error_t *err)
152+
{
135153
const char *p;
136154

137155
p = name;
@@ -141,39 +159,63 @@ get_seccomp_action (const char *name, int errno_ret, libcrun_error_t *err)
141159
p += 9;
142160

143161
if (strcmp (p, "ALLOW") == 0)
144-
return SCMP_ACT_ALLOW;
162+
{
163+
*action = SCMP_ACT_ALLOW;
164+
return 0;
165+
}
145166
else if (strcmp (p, "ERRNO") == 0)
146-
return SCMP_ACT_ERRNO (errno_ret);
167+
{
168+
*action = SCMP_ACT_ERRNO (errno_ret);
169+
return 0;
170+
}
147171
else if (strcmp (p, "KILL") == 0)
148-
return SCMP_ACT_KILL;
172+
{
173+
*action = SCMP_ACT_KILL;
174+
return 0;
175+
}
149176
# ifdef SCMP_ACT_LOG
150177
else if (strcmp (p, "LOG") == 0)
151-
return SCMP_ACT_LOG;
178+
{
179+
*action = SCMP_ACT_LOG;
180+
return 0;
181+
}
152182
# endif
153183
else if (strcmp (p, "TRAP") == 0)
154-
return SCMP_ACT_TRAP;
184+
{
185+
*action = SCMP_ACT_TRAP;
186+
return 0;
187+
}
155188
else if (strcmp (p, "TRACE") == 0)
156-
return SCMP_ACT_TRACE (errno_ret);
189+
{
190+
*action = SCMP_ACT_TRACE (errno_ret);
191+
return 0;
192+
}
157193
# ifdef SCMP_ACT_KILL_PROCESS
158194
else if (strcmp (p, "KILL_PROCESS") == 0)
159-
return SCMP_ACT_KILL_PROCESS;
195+
{
196+
*action = SCMP_ACT_KILL_PROCESS;
197+
return 0;
198+
}
160199
# endif
161200
# ifdef SCMP_ACT_KILL_THREAD
162201
else if (strcmp (p, "KILL_THREAD") == 0)
163-
return SCMP_ACT_KILL_THREAD;
202+
{
203+
*action = SCMP_ACT_KILL_THREAD;
204+
return 0;
205+
}
164206
# endif
165207
# ifdef SCMP_ACT_NOTIFY
166208
else if (strcmp (p, "NOTIFY") == 0)
167-
return SCMP_ACT_NOTIFY;
209+
{
210+
*action = SCMP_ACT_NOTIFY;
211+
return 0;
212+
}
168213
# endif
169214

170215
fail:
171-
crun_make_error (err, 0, "seccomp get action `%s`", name);
172-
return 0;
173-
#else
174-
return 0;
175-
#endif
216+
return crun_make_error (err, 0, "seccomp get action `%s`", name);
176217
}
218+
#endif
177219

178220
static void
179221
make_lowercase (char *str)
@@ -666,7 +708,8 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err
666708
int ret;
667709
size_t i;
668710
cleanup_seccomp scmp_filter_ctx ctx = NULL;
669-
int action, default_action, default_errno_value = EPERM;
711+
int default_errno_value = EPERM;
712+
uint32_t action, default_action;
670713
const char *def_action = NULL;
671714

672715
/* The bpf filter was loaded from the cache, nothing to do here. */
@@ -696,9 +739,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err
696739
default_errno_value = seccomp->default_errno_ret;
697740
}
698741

699-
default_action = get_seccomp_action (def_action, default_errno_value, err);
700-
if (UNLIKELY (err && *err != NULL))
701-
return -1;
742+
ret = get_seccomp_action (def_action, default_errno_value, &default_action, err);
743+
if (UNLIKELY (ret < 0))
744+
return ret;
702745

703746
ctx = seccomp_init (default_action);
704747
if (ctx == NULL)
@@ -741,9 +784,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err
741784
errno_ret = seccomp->syscalls[i]->errno_ret;
742785
}
743786

744-
action = get_seccomp_action (seccomp->syscalls[i]->action, errno_ret, err);
745-
if (UNLIKELY (err && *err != NULL))
746-
return -1;
787+
ret = get_seccomp_action (seccomp->syscalls[i]->action, errno_ret, &action, err);
788+
if (UNLIKELY (ret < 0))
789+
return ret;
747790

748791
if (action == default_action)
749792
continue;
@@ -795,9 +838,9 @@ libcrun_generate_seccomp (struct libcrun_seccomp_gen_ctx_s *gen_ctx, libcrun_err
795838
char *op = seccomp->syscalls[i]->args[k]->op;
796839

797840
arg_cmp[k].arg = seccomp->syscalls[i]->args[k]->index;
798-
arg_cmp[k].op = get_seccomp_operator (op, err);
799-
if (arg_cmp[k].op == 0)
800-
return crun_make_error (err, 0, "get_seccomp_operator");
841+
ret = get_seccomp_operator (op, &(arg_cmp[k].op), err);
842+
if (UNLIKELY (ret < 0))
843+
return ret;
801844
arg_cmp[k].datum_a = seccomp->syscalls[i]->args[k]->value;
802845
arg_cmp[k].datum_b = seccomp->syscalls[i]->args[k]->value_two;
803846
}

0 commit comments

Comments
 (0)