| Message ID | 20240719161713.963130-1-vmojzis@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | libsepol/sepol_compute_sid: Do not destroy uninitialized context | expand |
On Fri, 19 Jul 2024 at 18:17, Vit Mojzis <vmojzis@redhat.com> wrote: > > Avoid context_destroy() on "newcontext" before context_init() is called. > > Fixes: > libsepol-3.6/src/services.c:1335: var_decl: Declaring variable "newcontext" without initializer. > libsepol-3.6/src/services.c:1462: uninit_use_in_call: Using uninitialized value "newcontext.range.level[0].cat.node" when calling "context_destroy". > \# 1460| rc = sepol_sidtab_context_to_sid(sidtab, &newcontext, out_sid); > \# 1461| out: > \# 1462|-> context_destroy(&newcontext); > \# 1463| return rc; > \# 1464| } > > Signed-off-by: Vit Mojzis <vmojzis@redhat.com> LGTM. Reviewed-by: Christian Göttsche <cgzones@googlemail.com> > --- > libsepol/src/services.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/libsepol/src/services.c b/libsepol/src/services.c > index 36e2368f..f3231f17 100644 > --- a/libsepol/src/services.c > +++ b/libsepol/src/services.c > @@ -1362,14 +1362,12 @@ static int sepol_compute_sid(sepol_security_id_t ssid, > scontext = sepol_sidtab_search(sidtab, ssid); > if (!scontext) { > ERR(NULL, "unrecognized SID %d", ssid); > - rc = -EINVAL; > - goto out; > + return -EINVAL; > } > tcontext = sepol_sidtab_search(sidtab, tsid); > if (!tcontext) { > ERR(NULL, "unrecognized SID %d", tsid); > - rc = -EINVAL; > - goto out; > + return -EINVAL; > } > > if (tclass && tclass <= policydb->p_classes.nprim) > -- > 2.43.0 > >
On Fri, Jul 19, 2024 at 12:17 PM Vit Mojzis <vmojzis@redhat.com> wrote: > > Avoid context_destroy() on "newcontext" before context_init() is called. > > Fixes: > libsepol-3.6/src/services.c:1335: var_decl: Declaring variable "newcontext" without initializer. > libsepol-3.6/src/services.c:1462: uninit_use_in_call: Using uninitialized value "newcontext.range.level[0].cat.node" when calling "context_destroy". > \# 1460| rc = sepol_sidtab_context_to_sid(sidtab, &newcontext, out_sid); > \# 1461| out: > \# 1462|-> context_destroy(&newcontext); > \# 1463| return rc; > \# 1464| } > > Signed-off-by: Vit Mojzis <vmojzis@redhat.com> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> BTW, this function has long since diverged from the corresponding kernel function security_compute_sid; originally they were identical and even built from the same sources but we forked them long ago to specialize the kernel code. Don't believe anything is using it except for checkpolicy (via the -d option for the transition/member/change_sid commands) but no one should be relying on it matching the kernel's behavior.
On Tue, Jul 30, 2024 at 8:45 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > On Fri, Jul 19, 2024 at 12:17 PM Vit Mojzis <vmojzis@redhat.com> wrote: > > > > Avoid context_destroy() on "newcontext" before context_init() is called. > > > > Fixes: > > libsepol-3.6/src/services.c:1335: var_decl: Declaring variable "newcontext" without initializer. > > libsepol-3.6/src/services.c:1462: uninit_use_in_call: Using uninitialized value "newcontext.range.level[0].cat.node" when calling "context_destroy". > > \# 1460| rc = sepol_sidtab_context_to_sid(sidtab, &newcontext, out_sid); > > \# 1461| out: > > \# 1462|-> context_destroy(&newcontext); > > \# 1463| return rc; > > \# 1464| } > > > > Signed-off-by: Vit Mojzis <vmojzis@redhat.com> > > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > BTW, this function has long since diverged from the corresponding > kernel function security_compute_sid; originally they were identical > and even built from the same sources but we forked them long ago to > specialize the kernel code. Don't believe anything is using it except > for checkpolicy (via the -d option for the > transition/member/change_sid commands) but no one should be relying on > it matching the kernel's behavior. Applied.
diff --git a/libsepol/src/services.c b/libsepol/src/services.c index 36e2368f..f3231f17 100644 --- a/libsepol/src/services.c +++ b/libsepol/src/services.c @@ -1362,14 +1362,12 @@ static int sepol_compute_sid(sepol_security_id_t ssid, scontext = sepol_sidtab_search(sidtab, ssid); if (!scontext) { ERR(NULL, "unrecognized SID %d", ssid); - rc = -EINVAL; - goto out; + return -EINVAL; } tcontext = sepol_sidtab_search(sidtab, tsid); if (!tcontext) { ERR(NULL, "unrecognized SID %d", tsid); - rc = -EINVAL; - goto out; + return -EINVAL; } if (tclass && tclass <= policydb->p_classes.nprim)
Avoid context_destroy() on "newcontext" before context_init() is called. Fixes: libsepol-3.6/src/services.c:1335: var_decl: Declaring variable "newcontext" without initializer. libsepol-3.6/src/services.c:1462: uninit_use_in_call: Using uninitialized value "newcontext.range.level[0].cat.node" when calling "context_destroy". \# 1460| rc = sepol_sidtab_context_to_sid(sidtab, &newcontext, out_sid); \# 1461| out: \# 1462|-> context_destroy(&newcontext); \# 1463| return rc; \# 1464| } Signed-off-by: Vit Mojzis <vmojzis@redhat.com> --- libsepol/src/services.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)