bin/141692: Segmentation fault in jls -jJNAME

[Markiyan Kushnir]

>Number:         141692
>Category:       bin
>Synopsis:       Segmentation fault in jls -jJNAME
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Dec 16 21:30:04 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Markiyan Kushnir
>Release:        8.0-STABLE
>Organization:
Lohika Systems
>Environment:
FreeBSD localhost 8.0-STABLE FreeBSD 8.0-STABLE #2: Fri Dec 11 00:54:35 EET 2009     root at localhost:/usr/obj/usr/src/sys/MAREK  i386

>Description:
jls(8) with a jail name (not jid) supplied using the -j option, regardless of whether it is existing name or not, fails with a segfault. After some code review of lib/libjail/jail.c, found an attempt of possible NULL pointer dereference, lines 534-535 (v 1.3.2.1). A workaround is proposed to "preventively" supply a valid buffer for the jid parameter in jls.c
>How-To-Repeat:
jls -j asdasd

Or compile with -ljail this simple demo:

#include <stdlib.h>
#include <stdio.h>
#include <sys/param.h>
#include <sys/jail.h>
#include <jail.h>
int
main (void)
{
  char * hostname;
  struct jailparam params[3];
  int res;

  if (jailparam_init(&params[0], "name") != 0) {
    perror("jailparam_init name");
  }
  /* setup name as a key parameter */
  if (jailparam_import(&params[0], "asdf") != 0) {
    perror("jailparam_import asdf");
  }

  if (jailparam_init(&params[1], "host.hostname") != 0) {
    perror("jailparam_init host.hostname");
  }

  /* jid is not the key parameter */
  if (jailparam_init(&params[2], "jid") != 0) {
    perror("jailparam_init");
  }

  if ((res = jailparam_get(params, 3, 0)) == -1) {
    perror("jailparam_get");
  }

  hostname = jailparam_export(&params[1]);
  printf("hostname='%s'\n", hostname);
  jailparam_free(params, 3);
  return 0;
}

>Fix:
The library function jailparam_get(3) makes an assumption that only jid or lastjid can be key parameters. No such condition is mentioned in the man 3 jail.

The workaround to jls is proposed in the attachment. True fix would require a bit more research in the libjail.

Patch attached with submission follows:

--- /usr/src/usr.sbin/jls/jls.c	2009-08-12 15:31:29.000000000 +0300
+++ jls.c	2009-12-16 22:27:13.000000000 +0200
@@ -115,7 +115,7 @@
 		if (pflags & (PRINT_HEADER | PRINT_NAMEVAL))
 			add_param("all", NULL, (size_t)0, NULL, JP_USER);
 		else if (pflags & PRINT_VERBOSE) {
-			add_param("jid", NULL, (size_t)0, NULL, JP_USER);
+			add_param("jid", &jid, sizeof(jid), NULL, JP_USER);
 			add_param("host.hostname", NULL, (size_t)0, NULL,
 			    JP_USER);
 			add_param("path", NULL, (size_t)0, NULL, JP_USER);
@@ -127,7 +127,7 @@
 			    JP_USER | JP_OPT);
 		} else {
 			pflags |= PRINT_DEFAULT;
-			add_param("jid", NULL, (size_t)0, NULL, JP_USER);
+			add_param("jid", &jid, sizeof(jid), NULL, JP_USER);
 			add_param("ip4.addr", NULL, (size_t)0, NULL, JP_USER);
 			add_param("host.hostname", NULL, (size_t)0, NULL,
 			    JP_USER);


>Release-Note:
>Audit-Trail:
>Unformatted: