Get best of the week

Y nuevamente sobre incrustado: buscando errores en el proyecto Embox

Figura 2

Embox es un sistema operativo multiplataforma y multitarea en tiempo real para sistemas integrados. Está diseñado para funcionar en recursos informáticos bajos y le permite ejecutar aplicaciones basadas en Linux en microcontroladores sin usar Linux. Por supuesto, como cualquier otra aplicación, los errores de Embox tampoco están exentos. Este artículo está dedicado al análisis de errores encontrados en el código del proyecto Embox.

Hace unos meses, ya escribí un artículo sobre la verificación de FreeRTOS, otro sistema operativo para sistemas integrados. Entonces no encontré errores, pero los encontré en bibliotecas agregadas por los chicos de Amazon al desarrollar su propia versión de FreeRTOS.

El artículo que ves ahora frente a ti, en cierto sentido, continúa con el tema del anterior. A menudo recibimos solicitudes para verificar FreeRTOS, y lo hicimos. Esta vez, no hubo solicitudes para verificar un proyecto específico, pero comencé a recibir cartas y comentarios de desarrolladores integrados a quienes les gustó y quieren más.

Bueno, el nuevo número de PVS-Studio para la revista Embedded ha madurado y yace ante ti. ¡Disfruto ver!

Análisis


El análisis se llevó a cabo utilizando PVS-Studio, un analizador de código estático para C, C ++, C # y Java. Antes del análisis, el proyecto debe ensamblarse, por lo que nos aseguraremos de que el código del proyecto funcione y también le daremos al analizador la oportunidad de recopilar la información de ensamblaje que puede ser útil para una mejor verificación del código.

Las instrucciones en el repositorio oficial de Embox ofrecen la capacidad de construir bajo diferentes sistemas (Arch Linux, macOS, Debian) y usar Docker. Decidí agregar algo de variedad a mi vida y construir y analizar desde Debian, que instalé recientemente en mi máquina virtual.

La asamblea fue sin problemas. Ahora era necesario realizar un análisis. Debian es uno de los sistemas basados ​​en Linux PVS-Studio compatibles. Una forma conveniente de verificar proyectos desde Linux es compilar el seguimiento. Este es un modo especial en el que el analizador recopila toda la información necesaria sobre el ensamblaje para que luego pueda comenzar el análisis con un solo clic. Todo lo que necesitaba hacer era:

1) Descargar e instalar PVS-Studio;

2) Inicie el seguimiento del ensamblaje yendo a la carpeta con Embox y escribiendo en el terminal

pvs-studio-analyzer analyze -- make

3) Después de esperar a que se complete el ensamblaje, ejecute el comando:

pvs-studio-analyzer analyze -o /path/to/output.log

4) Convierta el informe sin formato a cualquier formato conveniente para usted. El analizador viene con una utilidad especial PlogConverter, con la que puede hacer esto. Por ejemplo, el comando para convertir el informe en una lista de tareas (para ver, por ejemplo, en QtCreator) se verá así:

plog-converter -t tasklist -o /path/to/output.tasks /path/to/project

¡Todas! No me llevó más de 15 minutos completar estos puntos. El informe está listo, ahora puede ver los errores. Bueno, empecemos :)

Ciclo extraño


Uno de los errores encontrados por el analizador fue el extraño ciclo while:

int main(int argc, char **argv) {
  ....

  while (dp.skip != 0 ) {
    n_read = read(ifd, tbuf, dp.bs);
    if (n_read < 0) {
      err = -errno;
      goto out_cmd;
    }
    if (n_read == 0) {
      goto out_cmd;
    }

    dp.skip --;
  } while (dp.skip != 0);       // <=

  do {
    n_read = read(ifd, tbuf, dp.bs);
    if (n_read < 0) {
      err = -errno;
      break;
    }

    if (n_read == 0) {
      break;
    }

    ....

    dp.count --;
  } while (dp.count != 0);
  ....
}

Advertencia de PVS-Studio: V715 El operador 'while' tiene el cuerpo vacío. Patrón sospechoso detectado: 'while (expr) {...} while (dp.skip! = 0);'. dd.c 225 hm

. Realmente extraño bucle. La expresión while (dp.skip! = 0) se escribe dos veces, una justo encima del bucle y la segunda justo debajo. De hecho, ahora estos son dos ciclos diferentes: uno contiene expresiones entre llaves y el segundo está vacío. En este caso, el segundo ciclo nunca se ejecutará.

A continuación se muestra un bucle do ... while con una condición similar, lo que me lleva a pensar: el bucle extraño fue originalmente como do ... while, pero algo salió mal. Creo que este código con una alta probabilidad contiene un error lógico.

Pérdidas de memoria


Sí, no sin ellos.

int krename(const char *oldpath, const char *newpath) {
  
  char *newpatharg, *oldpatharg;

  ....

  oldpatharg =
    calloc(strlen(oldpath) + diritemlen + 2, sizeof(char));
  newpatharg =
    calloc(strlen(newpath) + diritemlen + 2, sizeof(char));
  if (NULL == oldpatharg || NULL == newpatharg) {
    SET_ERRNO(ENOMEM);
    return -1;
  }

  ....
}

Advertencias de PVS-Studio:

  • V773 The function was exited without releasing the 'newpatharg' pointer. A memory leak is possible. kfsop.c 611
  • V773 The function was exited without releasing the 'oldpatharg' pointer. A memory leak is possible. kfsop.c 611

La función crea las variables locales newpatharg y oldpatharg dentro de sí misma . A estos punteros se les asignan las direcciones de las nuevas ubicaciones de memoria asignadas internamente mediante calloc . Si se produce un problema al asignar memoria, calloc devuelve un puntero nulo.

¿Qué sucede si solo se asigna un bloque de memoria? La función se bloqueará sin liberar ninguna memoria. El sitio que se asignó permanecerá en la memoria sin ninguna oportunidad de acceder nuevamente y liberarlo para su uso posterior.

Otro ejemplo de pérdida de memoria es un poco más brillante:

static int block_dev_test(....) {
  int8_t *read_buf, *write_buf;
  
  ....

  read_buf = malloc(blk_sz * m_blocks);
  write_buf = malloc(blk_sz * m_blocks);

  if (read_buf == NULL || write_buf == NULL) {
    printf("Failed to allocate memory for buffer!\n");

    if (read_buf != NULL) {
      free(read_buf);
    }

    if (write_buf != NULL) {
      free(write_buf);
    }

    return -ENOMEM;
  }

  if (s_block >= blocks) {
    printf("Starting block should be less than number of blocks\n");
    return -EINVAL;            // <=
  }

  ....
}

Advertencias de PVS-Studio:

  • V773 The function was exited without releasing the 'read_buf' pointer. A memory leak is possible. block_dev_test.c 195
  • V773 The function was exited without releasing the 'write_buf' pointer. A memory leak is possible. block_dev_test.c 195

Aquí, el programador ya ha mostrado precisión y procesó correctamente el caso en el que fue posible asignar solo una pieza de memoria. Procesado correctamente ... y literalmente en la siguiente expresión cometió otro error.

Gracias a una verificación escrita correctamente, podemos estar seguros de que en el momento en que se ejecuta la expresión, devuelve -EINVAL; definitivamente tendremos memoria asignada tanto para read_buf como write_buf . Por lo tanto, con tal retorno de la función, tendremos dos fugas a la vez.

Creo que tener una pérdida de memoria en un dispositivo integrado puede ser más doloroso que en una PC clásica. En condiciones en que los recursos son muy limitados, debe controlarlos con especial cuidado.

Punteros mal manejados


El siguiente código de error es conciso y lo suficientemente simple:

static int scsi_write(struct block_dev *bdev, char *buffer,
                      size_t count, blkno_t blkno) {
  struct scsi_dev *sdev;
  int blksize;

  ....

  sdev = bdev->privdata;
  blksize = sdev->blk_size; // <=

  if (!sdev) {              // <=
    return -ENODEV;
  }

  ....
}

Advertencia de PVS-Studio: V595 El puntero 'sdev' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 116, 118. scsi_disk.c 116

El puntero sdev se desreferencia justo antes de que se verifique NULL. Es lógico suponer que si alguien escribió dicho cheque, entonces este puntero puede ser nulo. En este caso, tenemos la posible desreferenciación del puntero nulo en la cadena blksize = sdev-> blk_size .

El error es que la verificación no se encuentra donde se necesita. Debería haber venido después de la línea " sdev = bdev-> privdata; ", pero antes de la línea " blksize = sdev-> blk_size; ". Entonces se podría evitar la posible apelación a la dirección cero.

PVS-Studio encontró dos errores más en el siguiente código:

void xdrrec_create(....)
{
  char *buff;

  ....

  buff = (char *)malloc(sendsz + recvsz);
  assert(buff != NULL);

  ....

  xs->extra.rec.in_base = xs->extra.rec.in_curr = buff;
  xs->extra.rec.in_boundry 
    = xs->extra.rec.in_base + recvsz;                    // <=

  ....
  xs->extra.rec.out_base
    = xs->extra.rec.out_hdr = buff + recvsz;             // <= 
  xs->extra.rec.out_curr 
    = xs->extra.rec.out_hdr + sizeof(union xdrrec_hdr);

  ....
}

Advertencias de PVS-Studio:

  • V769 El puntero 'xs-> extra.rec.in_base' en la expresión 'xs-> extra.rec.in_base + recvsz' podría ser nullptr. En tal caso, el valor resultante no tendrá sentido y no debe usarse. Líneas de verificación: 56, 48. xdr_rec.c 56
  • V769 El puntero 'buff' en la expresión 'buff + recvsz' podría ser nullptr. En tal caso, el valor resultante no tendrá sentido y no debe usarse. Líneas de verificación: 61, 48. xdr_rec.c 61

El puntero buf se inicializa con malloc , y luego su valor se usa para inicializar otros punteros. La función malloc puede devolver un puntero nulo, y esto siempre debe verificarse. Parecería que aquí es una aserción comprobar buf de NULL , y todo debería funcionar bien.

¡Pero no! El hecho es que las aserciones se utilizan para la depuración, y al compilar el proyecto en la configuración de lanzamiento, esta aserción se eliminará. Resulta que cuando se trabaja en Debug, el programa funcionará correctamente, y al compilar en Release, el puntero nulo "va" más allá.

Usar nuloen operaciones aritméticas es incorrecto, porque el resultado de tal operación no tendrá ningún sentido, y dicho resultado no puede usarse. De esto nos advierte el analizador.

Algunos podrían argumentar que no verificar el puntero después de malloc / realloc / calloc no tiene miedo. Como, en el primer acceso por un puntero nulo, se producirá una señal / excepción y no habrá nada malo. En la práctica, todo es mucho más complicado, y si la falta de verificación no le parece peligrosa, le sugiero que lea el artículo " ¿Por qué es importante verificar qué regresó la función malloc? ".

Manejo incorrecto de matrices


El siguiente error es muy similar al ejemplo anterior:


int fat_read_filename(struct fat_file_info *fi,
                      void *p_scratch,
                      char *name) {
  int offt = 1;

  ....

  offt = strlen(name);
  while (name[offt - 1] == ' ' && offt > 0) { // <=
    name[--offt] = '\0';
  }
  log_debug("name(%s)", name);

  return DFS_OK;
}

Advertencia de PVS-Studio: V781 El valor del índice 'offt' se verifica después de su uso. Quizás haya un error en la lógica del programa. fat_common.c 1813 La

variable offt se usa primero dentro de la operación de indexación, y solo entonces se verifica que su valor sea mayor que cero. Pero, ¿qué sucede si el nombre resulta ser una cadena vacía? La función strlen () devolverá 0 , luego un golpe fuerte en la pierna. El programa accederá a un índice negativo, lo que conducirá a un comportamiento indefinido. Cualquier cosa puede suceder, incluido un bloqueo del programa. ¡Lío!

Foto 1

Condiciones sospechosas


¿Y dónde sin ellos? Encontramos tales errores literalmente en cada proyecto que verificamos.

int index_descriptor_cloexec_set(int fd, int cloexec) {
  struct idesc_table *it;

  it = task_resource_idesc_table(task_self());
  assert(it);

  if (cloexec | FD_CLOEXEC) {
    idesc_cloexec_set(it->idesc_table[fd]);
  } else {
    idesc_cloexec_clear(it->idesc_table[fd]);
  }
  return 0;
}

Advertencia de PVS-Studio: V617 Considere inspeccionar la condición. El argumento '0x0010' de '|' La operación bit a bit contiene un valor distinto de cero. index_descriptor.c 55

Para comprender en qué consiste el error, observe la definición de la constante FD_CLOEXEC :

#define FD_CLOEXEC 0x0010

Resulta que en la expresión if (cloexec | FD_CLOEXEC) a la derecha del bit a bit "o" siempre hay una constante distinta de cero. El resultado de dicha operación siempre será un número distinto de cero. Por lo tanto, esta expresión siempre será equivalente a la expresión if (verdadero) , y siempre procesaremos solo la rama entonces de la expresión if.

Sospecho que esta constante de macro se usa para preconfigurar el sistema operativo Embox, pero aun así esta condición siempre verdadera parece extraña. Quizás quisieron usar el operador & aquí , pero cometieron un error tipográfico.

División entera


El siguiente error está relacionado con una característica del lenguaje C:

#define SBSIZE    1024

static int ext2fs_format(struct block_dev *bdev, void *priv) {
  size_t dev_bsize;
  float dev_factor;

  ....

  dev_size = block_dev_size(bdev);
  dev_bsize = block_dev_block_size(bdev);
  dev_factor = SBSIZE / dev_bsize;            // <=

  ext2_dflt_sb(&sb, dev_size, dev_factor);
  ext2_dflt_gd(&sb, &gd);

  ....
}

Advertencia de PVS-Studio: V636 La expresión '1024 / dev_bsize' se convirtió implícitamente de tipo 'int' a tipo 'flotante'. Considere utilizar un molde de tipo explícito para evitar la pérdida de una parte fraccional. Un ejemplo: doble A = (doble) (X) / Y;. ext2.c 777

Esta característica es la siguiente: si dividimos dos valores enteros, entonces el resultado de la división será entero. Por lo tanto, la división ocurrirá sin un resto, o, en otras palabras, la parte fraccionaria se descartará del resultado de la división.

A veces los programadores lo olvidan y se cometen errores como este. SBSIZE constante y variable dev_bsize tienen un tipo entero (int y size_t respectivamente). Por lo tanto, el resultado de la expresión SBSIZE / dev_bsizetambién será de tipo entero.

Pero espera un momento. La variable dev_factor es de tipo float ! Obviamente, el programador esperaba obtener un resultado de división fraccional. Esto puede verificarse aún más si presta atención al uso posterior de esta variable. Por ejemplo, la función ext2_dflt_sb , donde dev_factor se pasa como el tercer parámetro, tiene la siguiente firma:

static void ext2_dflt_sb(struct ext2sb *sb, size_t dev_size, float dev_factor);

De manera similar, en otros lugares donde se usa la variable dev_factor : todo indica que se espera un número de coma flotante.

Para corregir este error, es suficiente convertir uno de los operandos de división a un tipo real. Por ejemplo:

dev_factor = float(SBSIZE) / dev_bsize;

Entonces el resultado de la división será un número fraccionario.

Entrada no verificada


El siguiente error está asociado con el uso de datos no verificados recibidos desde fuera del programa.

int main(int argc, char **argv) {
  int ret;
  char text[SMTP_TEXT_LEN + 1];

  ....

  if (NULL == fgets(&text[0], sizeof text - 2, /* for \r\n */
      stdin)) { ret = -EIO; goto error; }
    text[strlen(&text[0]) - 1] = '\0'; /* remove \n */    // <=

  ....
}

Advertencia de PVS-Studio: V1010 Los datos contaminados no verificados se usan en el índice: 'strlen (& text [0])'. sendmail.c 102

Primero, considere lo que devuelve la función fgets . En caso de lectura exitosa de una línea, la función devuelve un puntero a esta línea. Si la lectura encuentra un final de archivo antes de que se lea al menos un elemento, o si se produce un error de entrada, la función fgets devuelve NULL .

Entonces la expresión es NULL == fgets (....)comprueba si la entrada recibida es correcta. Pero hay una advertencia. Si transfiere un terminal nulo como el primer carácter que se leerá (esto se puede hacer, por ejemplo, presionando Ctrl + 2 en el modo Legacy de la línea de comando de Windows), la función fgets lo considera sin devolver NULL . En este caso, la línea en la que se realiza la grabación tendrá solo un elemento: ' \ 0 '.

¿Qué pasará después? La expresión strlen (& text [0]) devolverá 0 . Como resultado, recibimos una llamada de índice negativa:

text[ 0 - 1 ] = '\0';

Como resultado, podemos "anular" el programa simplemente pasando el carácter de terminación de línea a la entrada. Esto es incómodo y podría usarse potencialmente para atacar sistemas que usan Embox.

Mi colega, que estaba desarrollando esta regla de diagnóstico, incluso tomó nota con un ejemplo de tal ataque al proyecto NcFTP:


Recomiendo ver si aún no crees en esa oportunidad :)

Además, el analizador encontró dos lugares más con el mismo error:

  • V1010 Los datos contaminados no verificados se usan en el índice: 'strlen (& from [0])'. sendmail.c 55
  • V1010 Los datos contaminados no verificados se usan en el índice: 'strlen (& to [0])'. sendmail.c 65

Misra


MISRA es un conjunto de pautas y pautas para escribir código C y C ++ seguro para sistemas integrados altamente responsables. En cierto sentido, este es un manual de capacitación, que le permitirá deshacerse no solo de los llamados "olores de código", sino que también protegerá su programa de vulnerabilidades.

MISRA se utiliza donde las vidas humanas dependen de la calidad de su sistema integrado: en las industrias médica, automotriz, aeronáutica y militar.

PVS-Studio tiene un extenso conjunto de reglas de diagnóstico que le permiten verificar que su código cumpla con los estándares MISRA C y MISRA C ++. De manera predeterminada, el modo con estos diagnósticos está desactivado, pero como estamos buscando errores en el proyecto para sistemas integrados, simplemente no podría prescindir de MISRA.

Esto es lo que pude encontrar:

/* find and read symlink file */
static int ext2_read_symlink(struct nas *nas,
                             uint32_t parent_inumber,
                             const char **cp) {
  char namebuf[MAXPATHLEN + 1];

  ....

  *cp = namebuf;              // <=
  if (*namebuf != '/') {
    inumber = parent_inumber;
  } else {
    inumber = (uint32_t) EXT2_ROOTINO;
  }
  rc = ext2_read_inode(nas, inumber);

  return rc;
}

Advertencia de PVS-Studio: V2548 [MISRA C 18.6] La dirección de la matriz local 'namebuf' no debe almacenarse fuera del alcance de esta matriz. ext2.c 298

El analizador detectó una asignación sospechosa que podría conducir a un comportamiento indefinido.

Echemos un vistazo más de cerca al código. Aquí, namebuf es una matriz creada en el ámbito local de la función, y el puntero cp se pasa a la función por puntero.

Según la sintaxis de C, el nombre de la matriz es un puntero al primer elemento en el área de memoria en la que se almacena la matriz. Resulta que la expresión * cp = namebuf asignará la dirección de la matriz namebuf a la variable señalada por cp. Como cp se pasa a la función por puntero, un cambio en el valor al que apunta se reflejará en el lugar donde se llamó a la función.

Resulta que después de que la función ext2_read_symlink complete su trabajo, su tercer parámetro indicará el área que la matriz namebuf una vez ocupó .

Solo hay un "pero": dado que namebuf es una matriz reservada en la pila, se eliminará cuando salga la función. Por lo tanto, un puntero que existe fuera de la función apuntará a la memoria liberada.

¿Qué habrá en esa dirección? Es imposible de predecir. Es posible que durante algún tiempo el contenido de la matriz continúe en la memoria, o es posible que el programa borre inmediatamente esta área con algo más. En general, acceder a dicha dirección devolverá un valor indefinido, y el uso de dicho valor es un error grave.

El analizador también encontró otro error con la misma advertencia:

  • V2548 [MISRA C 18.6] La dirección de la variable local 'dst_haddr' no debe almacenarse fuera del alcance de esta variable. net_tx.c 82

Figura 6

Conclusión


Me gustó trabajar con el proyecto Embox. A pesar de que no escribí todos los errores encontrados en el artículo, el número total de advertencias fue relativamente pequeño y, en general, el código del proyecto fue escrito con alta calidad. Por lo tanto, expreso mi gratitud a los desarrolladores, así como a aquellos que contribuyeron al proyecto en nombre de la comunidad. ¡Eres genial!

Aprovecho esta oportunidad para transmitir saludos a los desarrolladores de Tula. Creo que en San Petersburgo no hace mucho frío en este momento :)

Aquí es donde termina mi artículo. Espero que hayas disfrutado leyéndolo y que hayas encontrado algo nuevo para ti.

Si está interesado en PVS-Studio y desea verificar de forma independiente algún proyecto usándolo, descárguelo y pruébelo . Esto no tomará más de 15 minutos.



Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: George Gribkov. Acerca de incrustado nuevamente: búsqueda de errores en el proyecto Embox .

More articles:


All Articles