kAworu's avatar

The root, the root, the root is on fire

publié par kAworu
il y a presque deux ans

aaah sysctl(3) ouais je connais c'est facile

Grâce à lua-sysctl, une interface sysctl(3) pour lua, et stop, un port de htop pour FreeBSD qui utilise sysctl(3) à la place de /proc et /sys, j'ai beaucoup joué avec sysctl(3).

Pour comprendre comment sysctl(3) marche c'est loin d'être simple, la doc est inexistante, la seule manière est d'aller fouiller dans le code d'outils systèmes comme top(1), ps(1) & Co. Dans le tas, évidement sysctl(8) est la référence et la première source pour comprendre le fonctionnement de sysctl(3).

si pour toi, sysctl(3) et sysctl(8) on dirait la même chose, je t'invite à RTFM par ici.

Le but aujourd'hui n'est pas d'expliquer comment ça marche (si vraiment y a des intéressés je ferais un autre post, et pour les interessées des cours privés), mais de montrer un bug que j'ai trouvé dans sysctl(8).

Le bogue

set_IK()

C'est une fonction de conversion d'unité de température que l'on trouve dans src/sbin/sysctl/sysctl.c.

Elle prend str comme argument, la description de la température en Farenheit OU Celsius, par exemple "90.5C" pour 90.5° Celsius, ou encore "150F" pour 150° Farenheit. Elle converti en déci Kelvin et met la valeur dans l'entier pointé par l'argument val. La fonction retourne 0 lorsqu'une erreur est survenue (l'argument str est invalide) ou 1 quand tout c'est bien passé.

faites entrer L'accusé!

 1 static int
 2 set_IK(char *str, int *val)
 3 {
 4     float temp;
 5     int len, kelv;
 6     char *p, *endptr;
 7 
 8     if ((len = strlen(str)) == 0)
 9         return (0);
10     p = &str[len - 1];
11     if (*p == 'C' || *p == 'F') {
12         *p = '\0';
13         temp = strtof(str, &endptr);
14         if (endptr == str || *endptr != '\0')
15             return (0);
16         if (*p == 'F')
17             temp = (temp - 32) * 5 / 9;
18         kelv = temp * 10 + 2732;
19     } else {
20         kelv = (int)strtol(str, &endptr, 10);
21         if (endptr == str || *endptr != '\0')
22             return (0);
23     }
24     *val = kelv;
25     return (1);
26 }

sarkoz^Wset_IK, je te vois!

Ligne 11 et 12, si on regarde bien, on voit:

    if (*p == 'C' || *p == 'F') {
        *p = '\0';

Puis ligne 16:

        if (*p == 'F')

Donc on trouve que dans *p il y a l'unité (C pour Celsius, F pour Farenheit), on assigne \0 à *p, puis ligne 16 on test si l'unité est F (Farenheit) mais on a précédemment mis \0 dans *p!

Résultat: une valeur donnée en Farenheit est toujours interprétée comme une valeur en Celsius! C'est un problème important car si on donne "176F" (qui fait 80° Celsius) la fonction va retourner 176° Celsius (soit 4490 déci Kelvin), ce qui est beaucoup trop, et cette fonction est utilisée par exemple pour définir des températures limites du matériel.

infirmière! un sparadrap pour set_IK

Le fix le plus direct que j'ai utilisé à l'origine pour lua-sysctl est de simplement sauver l'unité (C ou F) dans un char, et de tester celui-ci après. Mais en faite, si on réflechi bien, on a pas besoin de modifier la valeur pointée par p, et donc même pas l'argument str. Le patch final ne modifie qu'une ligne de la logique de set_IK(), le reste ajoute l'attribut const à str et p. On va en faite "mieux" utiliser la valeur endptr qui est donnée par strtof(3). Voilà le patch final, qui a été backporté jusqu'à FreeBSD 6:

http://svn.freebsd.org/viewvc/base?view=revision&revision=198340

On a eu chaud

Entre les discussions, tests et patches pour finalement changer que quelques lignes ça a quand même pris du temps. En travaillant sur ce bug, j'ai réalisé comme les bug en C peuvent être bien plus vicieux à corriger que ceux dans d'autre langages. Le bug a certainement été introduit à l'origine parce que l'auteur ne maitrisait pas à 100% strtof(3), et le résultat est plutôt désastreux par rapport à l'erreur.

Le C, ça pardonne pas™

un commentaire

écrire un commentaire

  1. coursework il y a quatre mois coursework 's avatar

    article intéressant, merci pour l’affichage

écrire un commentaire:


(utilisé pour gravatar, ne sera pas affiché)



tu peux utiliser la syntaxe markdown :)