On Sun, Sep 28, 2025 at 03:21:38PM +1000, David Leonard wrote:
> On Sat, 27 Sep 2025, Osama Abdelkader wrote:
> > Fix FIXME comment in display_verbose() by adding null check for cterm
> > before calling puts(). This prevents a potential segfault when
> > xmalloc_ttyname(0) returns NULL.
> >
> > Fixes: potential segfault in sestatus -v mode when terminal name cannot be
> > determined
> > Signed-off-by: Osama Abdelkader
> > ---
> > selinux/sestatus.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/selinux/sestatus.c b/selinux/sestatus.c
> > index 098a4d189..cad44632c 100644
> > --- a/selinux/sestatus.c
> > +++ b/selinux/sestatus.c
> > @@ -131,8 +131,9 @@ static void display_verbose(void)
> > puts("\nFile contexts:");
> >
> > cterm = xmalloc_ttyname(0);
> > -//FIXME: if cterm == NULL, we segfault!??
> > - puts(cterm);
> > + if (cterm) {
> > + puts(cterm);
> > + }
>
> It looks like the style is no braces around single statement blocks.
>
> > if (cterm && lgetfilecon(cterm, &con) >= 0) {
> > printf(COL_FMT "%s\n", "Controlling term:", con);
> > if (ENABLE_FEATURE_CLEAN_UP)
>
> I thought the controlling terminal would be obtained with ctermid(), but
> https://github.com/SELinuxProject/selinux/blob/main/policycoreutils/sestatus/sestatus.c
> also uses ttyname(0).
>
> That SELinuxProject code also doesn't print anything between
> "File Contexts:" and "Controlling terminal:", so perhaps the best thing
> to do here is just delete the puts(cterm) altogether? It might have
> been leftover debugging...?
>
> --- a/selinux/sestatus.c
> +++ b/selinux/sestatus.c
> @@ -131,13 +131,13 @@ static void display_verbose(void)
> puts("\nFile contexts:");
>
> cterm = xmalloc_ttyname(0);
> -//FIXME: if cterm == NULL, we segfault!??
> - puts(cterm);
> if (cterm && lgetfilecon(cterm, &con) >= 0) {
> printf(COL_FMT "%s\n", "Controlling term:", con);
> if (ENABLE_FEATURE_CLEAN_UP)
> freecon(con);
> }
> + if (ENABLE_FEATURE_CLEAN_UP)
> + free(cterm);
>
> for (i = 0; fc[i] != NULL; i++) {
> struct stat stbuf;
>
>
>
Thanks for the review, I'm going to send v2.
BR,
Osama
___
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
