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 <[email protected]> > > --- > > 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
