Comment 19 for bug 1269731

TJ (tj) wrote :

Michael attached the conf files are my request based on my analysis of the SEGFAULT trace results:

Segfault happened at: 0x7f1191eb98d9: mov 0x20(%rax),%rsi

## Registers.txt shows:
rax 0x0 0
rbx 0x7f1192f18ed0 139713456475856
rcx 0xe 14
rdx 0x7f1192f18fc0 139713456476096
rsi 0x10 16
rdi 0xffffffffffffffff -1
rbp 0x7f11929f2440 0x7f11929f2440
rsp 0x7fff6f682d60 0x7fff6f682d60

## So %rax is 0 (NULL)

## Disassembly shows:

=> 0x7f1191eb98d9: mov 0x20(%rax),%rsi
   0x7f1191eb98dd: mov 0x10(%rax),%rdi
   0x7f1191eb98e1: callq 0x7f1191ea28a0

## Which represents job_class_get_registered (file->job->name, file->job->session)

#0 conf_file_serialise (file=file@entry=0x7f11929f2440) at conf.c:1633
  1628: * re-exec. This may change though immediately after re-exec
  1629: * when conf_reload() gets called.
  1630: *
  1631: * See job_class_serialise_all() for further details.
  1632: */
  1633: registered = job_class_get_registered (file->job->name,
  1634: file->job->session);
  1638:

## 0x20(%rax) and 0x10(%rax) are offsets into struct JobClass (from init/job_class.h):
typedef struct job_class {
  NihList entry;

## 0x10(%rax)
  char *name;
  char *path;
## 0x20(%rax)
  Session * session;

## On amd64 pointers are 8 bytes wide so 'name' and 'session' will be 0x10 bytes apart.
## %rax will be the pointer "file->job"
## which means it is unexpectedly NULL

## Looking for opportunities for that to happen leads to init/conf.c::conf_reload_path()

static int
conf_reload_path (ConfSource *source,
      const char *path,
      const char *override_path)
{
  ConfFile *file = NULL;
...
  /* Create a new ConfFile structure (if no @override_path specified) */
  file = (ConfFile *)nih_hash_lookup (source->files, path);
  if (! file)
    file = NIH_MUST (conf_file_new (source, path));

### at this point file->job should be NULL

 switch (source->type) {
...
  case CONF_JOB_DIR:

    name = conf_to_job_name (source->path, path);

    /* Create a new job item and parse the buffer to produce
     * the job definition.
     */
...
## the call to init/job_parse.c::parse_job() can return NULL

    file->job = parse_job (NULL, source->session, file->job,
        name, buf, len, &pos, &lineno);

    /* Allow the original ConfFile which has now been replaced to be
     * destroyed which will also cause the original JobClass to be
     * freed.
     */
    if (file->job) {
      job_class_consider (file->job);
    } else {
      err = nih_error_get ();
    }

## init/job_parse.c::parse_job() can return NULL:

JobClass *
parse_job (const void *parent,
     Session *session,
     JobClass *update,

## update is the local reference to file->job

     const char *name,
     const char *file,
     size_t len,
     size_t *pos,
     size_t *lineno)
{
  JobClass *class;

  nih_assert (name != NULL);
  nih_assert (file != NULL);
  nih_assert (pos != NULL);

  if (update) {
    class = update;
    nih_debug ("Reusing JobClass %s (%s)",
        class->name, class->path);
  } else {

## should be in this path if file->job == NULL

    nih_debug ("Creating new JobClass %s",
        name);
    class = job_class_new (parent, name, session);
    if (! class)
      nih_return_system_error (NULL);
  }

  if (nih_config_parse_file (file, len, pos, lineno,
        stanzas, class) < 0) {

## if parsing failed and file->job == NULL the new job is discarded

    if (!update)
      nih_free (class);

## and NULL is returned

    return NULL;
  }

  return class;
}