Journal D'un kernel panic à un patch…

Posté par . Licence CC by-sa
110
12
nov.
2017

Sommaire

Bonjour tout le monde

Ma distribution, Debian sid, m'a proposé de passer sur le noyau 4.13. À l'occasion d'un reboot, je tente un démarrage sur ce nouveau noyau… et suis accueilli par un kernel panic. Expérience déplaisante s'il en est, je décide de repasser sur le 4.12, du travail m'attendant.
Ayant eu plus de temps disponible ce week-end, je décide d'essayer le noyau 4.14-rc7, et découvre que le bug est toujours présent. Il est donc temps pour une enquête et un patch pour que les nouveaux noyaux continuent de démarrer mon système… Et on verra que des rudiments de C peuvent être suffisants pour certains bugs.

L'analyse du kernel panic

J'ai été contraint de prendre une photo du kernel panic (ma carte mère n'a pas le nécessaire pour sauvegarder un kernel panic hélas).
Le kernel panic est visible ici : http://host.pinaraf.info/~pinaraf/kernel-panic-4.13-crypto.jpg (notez le cadrage excellent de la photo… je l'ai prise très rapidement car un autre message d'erreur non lié allait apparaître)

Les éléments intéressants sont les suivants :

  • l'erreur en tant que telle
BUG: unable to handle kernel pointer dereference at 00000000000000xx
IP: ecc_gen_privkey+0xa9 [ecdh_generic]
  • la call trace,
  • les registres.

La call trace et l'IP (Instruction Pointer) montrent que l'erreur a lieu dans le module ecdh_generic, dans la fonction ecc_gen_privkey à l'adresse 0xa9 (relative au début de la fonction).
L'erreur est une déréférence de pointeur NULL.

Nous allons devoir regarder le code pour comprendre ce qu'il se passe dans ce module.

Les pré-requis pour analyser le plantage

J'ai installé les paquets suivants pour travailler à partir du noyau 4.13 : gdb, linux-image-4.13.0-1-amd64-dbg et linux-source-4.13.

Ces paquets sont TRÈS volumineux. Ils installent un ensemble de symboles de debug dans /usr/lib/debug pour le premier, et le second nous met à disposition dans /usr/src une archive du code du noyau avec les patchs correspondant de debian.
Je décompresse le code dans ma partition /data (les symboles de debug me prennent déjà 4GB, je peux pas beaucoup plus sur le SSD), et je dégaine gdb. Le module concerné est ecdh_generic, donc…

L'analyse du problème

Lançons GDB !

$ gdb /usr/lib/debug/lib/modules/4.13.0-1-amd64/kernel/crypto/ecdh_generic.ko
GNU gdb (Debian 7.12-6+b1) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/lib/debug/lib/modules/4.13.0-1-amd64/kernel/crypto/ecdh_generic.ko...done.

Il ne reste plus qu'à indiquer à gdb où se trouve le code, et lui demander de désassembler la fonction ecc_gen_privkey en indiquant le code source correspondant. On sait que l'erreur a lieu à l'offset 0xa9, soit 169 en décimal. Je ne vous mettrai donc que le code correspondant à cette partie ici.

(gdb) set directories /data/pierre/kernel-panic/linux-source-4.13/
(gdb) disassemble /s ecc_gen_privkey
Dump of assembler code for function ecc_gen_privkey:
./crypto/ecc.c:
…
949             int err;
950
951             /* Check that N is included in Table 1 of FIPS 186-4, section 6.1.1 */
952             if (nbits < 160)
   0x000000000000226d <+141>:   jbe    0x22c7 <ecc_gen_privkey+231>
   0x000000000000226f <+143>:   mov    %r9,-0x38(%rbp)

954
955             /*
956              * FIPS 186-4 recommends that the private key should be obtained from a
957              * RBG with a security strength equal to or greater than the security
958              * strength associated with N.
959              *
960              * The maximum security strength identified by NIST SP800-57pt1r4 for
961              * ECC is 256 (N >= 512).
962              *
963              * This condition is met by the default RNG because it selects a favored
964              * DRBG with a security strength of 256.
965              */
966             if (crypto_get_default_rng())
   0x0000000000002273 <+147>:   callq  0x2278 <ecc_gen_privkey+152>

967                     err = -EFAULT;
968
969             err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv, nbytes);
   0x0000000000002278 <+152>:   mov    0x0(%rip),%rdi        # 0x227f <ecc_gen_privkey+159>

./include/crypto/rng.h:
143             return crypto_rng_alg(tfm)->generate(tfm, src, slen, dst, dlen);
   0x000000000000227f <+159>:   mov    %r15d,%r8d
   0x0000000000002282 <+162>:   xor    %edx,%edx
   0x0000000000002284 <+164>:   xor    %esi,%esi
   0x0000000000002286 <+166>:   mov    %rbx,%rcx
   0x0000000000002289 <+169>:   mov    0x38(%rdi),%rax
   0x000000000000228d <+173>:   callq  *-0x20(%rax)
   0x0000000000002290 <+176>:   mov    %eax,%r15d

On voit bien à l'offset 169 une utilisation du registre RDI qui était à 0 dans le kernel panic. Le bug est donc assez évident : crypto_rng_alg(tfm) doit renvoyer un pointeur null, donc la méthode generate est à une adresse incorrecte, et la tentative d'appel échoue.

Le code d'ecc.c correspondant est le suivant :

        if (crypto_get_default_rng())
                err = -EFAULT;

        err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv, nbytes);

On note là une erreur de logique : si l'appel à crypto_get_default_rng échoue, on stocke une valeur dans une variable sans l'utiliser par la suite ? Cela semble incorrect.
Voici le code de crypto_get_default_rng :

int crypto_get_default_rng(void)
{
        struct crypto_rng *rng;
        int err;

        mutex_lock(&crypto_default_rng_lock);
        if (!crypto_default_rng) {
                rng = crypto_alloc_rng("stdrng", 0, 0);
                err = PTR_ERR(rng);
                if (IS_ERR(rng))
                        goto unlock;

                err = crypto_rng_reset(rng, NULL, crypto_rng_seedsize(rng));
                if (err) {
                        crypto_free_rng(rng);
                        goto unlock;
                }

                crypto_default_rng = rng;
        }

        crypto_default_rng_refcnt++;
        err = 0;

unlock:
        mutex_unlock(&crypto_default_rng_lock);

        return err;
}

Si cet appel échoue, la variable crypto_default_rng reste à NULL. Donc l'appel par la suite à crypto_rng_get_bytes avec un pointeur NULL déclenche la déréférence de pointeur NULL.

Le correctif est donc très simple : si crypto_get_default_rng renvoie une erreur, il faut renvoyer -EFAULT et pas juste ignorer l'erreur…

Correction du problème et envoi du patch…

Le bug a lieu dans la partie crypto du noyau. Regardons qui s'en occupe…

$ ./scripts/get_maintainer.pl crypto/ecc.c
Herbert Xu <herbert@gondor.apana.org.au> (maintainer:CRYPTO API)
"David S. Miller" <davem@davemloft.net> (maintainer:CRYPTO API)
linux-crypto@vger.kernel.org (open list:CRYPTO API)
linux-kernel@vger.kernel.org (open list)

Donc nous avons là la mailing list, les mainteneurs, et nous pouvons déduire de https://git.kernel.org/ qu'il faut cloner le dépôt https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git/ pour disposer du dernier code de cette partie du noyau.

Après un long git clone (le noyau, c'est beaucoup), on peut voir que le bug est toujours présent dans le dernier noyau.
On fait donc un patch de la ligne err = -EFAULT; pour la changer en return -EFAULT;… Et il n'y a plus qu'à préparer le tout.

$ git commit -sa
# Je vous laisse imaginer un beau message
$ git format-patch origin/master -o ../patchs

On a dans le dossier ../patchs un beau fichier .patch, qu'il n'y a plus qu'à envoyer aux adresses précédemment citées. (Paquet git-email pour les debianneux)

git send-email --to=linux-crypto@vger.kernel.org -cc=davem@davemloft.net -cc=herbert@gondor.apana.org.au ../patchs/0001-Fix-NULL-pointer-deref.-on-no-default_rng.patch

Et voilà !

J'espère que le 4.14 sera corrigé avant sa sortie, je n'aime pas avoir une version plus maintenue du noyau…

  • # Bravo et merci

    Posté par . Évalué à 10 (+19/-0).

    Merci pour ton travail, et merci pour ce journal l'expliquant clairement !
    :)

    Je note tout de même que tu sembles avoir bien plus que des rudiments de C, notamment de la motivation et une connaissance du processus de développement du noyau.

  • # Mail thread

    Posté par (page perso) . Évalué à 10 (+14/-0).

    Pour ceux qui veulent suivre la correction du patch, le lien vers l'archive de la mailing list: https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg29135.html.

    Et merci pour le journal.

    « Rappelez-vous toujours que si la Gestapo avait les moyens de vous faire parler, les politiciens ont, eux, les moyens de vous faire taire. » Coluche

  • # Mauvais timing...

    Posté par . Évalué à 6 (+4/-0).

    J'espère que le 4.14 sera corrigé avant sa sortie, je n'aime pas avoir une version plus maintenue du noyau…

    Le noyau 4.14 est sorti ( https://lwn.net/Articles/738810/rss ). Peut-être pour la prochaine version stable…

    • [^] # Re: Mauvais timing...

      Posté par . Évalué à 2 (+1/-0).

      Même pas dans des versions mineures ?

      • [^] # Re: Mauvais timing...

        Posté par (page perso) . Évalué à 4 (+1/-0).

        Cela est en effet possible vu qu'il semble que c'est juste une correction de bogue sans grand impact négatif (au contraire) sur les applications.

        Cela dépend des mainteneurs. D'ailleurs un rétroportage par ce biais est possible aussi sur les versions antérieures.

        • [^] # Re: Mauvais timing...

          Posté par . Évalué à 4 (+2/-0). Dernière modification le 15/11/17 à 15:58.

          Absolument, et cela est même très codifié. On peut lire tout cela dans le Kernel Development Process, que l'on trouve également au format texte (et maintenant *.rst) sous Documentation/process dans l'arborescence des sources du noyau.

          Le texte commence à avoir de l'âge mais on y lit notamment :

          Once a stable release is made, its ongoing maintenance is passed off to the
          "stable team," currently consisting of Greg Kroah-Hartman. The stable team
          will release occasional updates to the stable release using the 2.6.x.y
          numbering scheme. To be considered for an update release, a patch must (1)
          fix a significant bug, and (2) already be merged into the mainline for the
          next development kernel.

          Soit littéralement :

          Une fois la version stable publiée, son entretien est transmis à l'équipe « stable », actuellement formée d'un [seul] membre : Greg Kroah-Hartman. L'équipe stable publiera des mises à jour occasionnelles de la branche stable en utilisant la numérotation 2.6.x.y [aujourd'hui 4.x.y]. Pour envisager son intégration dans une mise à jour, un patch doit (1) corriger un bug non négligeable, et (2) avoir déjà été intégré dans la branche principale du développement du prochain noyau.

          Donc, concrètement, ça veut dire que quoi qu'il se passe, ton patch doit suivre la procédure normale, en passant par la branche de développement linux-next, pour vérifier entre autre qu'il compile bien et qu'il ne provoque pas d'autres régressions (ce qui serait un comble). Et cela signifie aussi qu'il doit d'abord inclus dans le noyau courant avant d'être backporté aux mises à jour des versions précédentes. On n'y pense pas mais ça tombe effectivement sous le sens une fois qu'on le dit : ce serait idiot de corriger un bug dans la version qui vient de sortir pour voir réapparaître le bug dans le noyau suivant…

          Tout cela est important parce que la phase d'intégration (de nouveautés ou autres) ne dure que deux semaines à chaque fois, suivies de SEPT semaines environ de phase de stabilisation : les fameuses -rc1 à -rc7 (le plus souvent car pour 4.14, on a eu une -rc8), chaque rc (release candidate) étant publiée par Linus le dimanche soir. Et quand on regarde l'arbre Git ainsi que les notes de publication sur la LKML, que l'on peut retrouver par exemple dans l'actuelle dépêche sur la sortie du noyau 4.14 en cours de rédaction, on s'aperçoit que nombreux sont les gens à soumettre des patches, voire des lignées entières de soumissions « urgent-pour-linus » le vendredi soir de la dernière semaine, ce qui a généralement le don de mettre le chef de mauvaise humeur. :)

          Tout cela pour dire que c'est cuit pour le 4.14 proprement dit, mais qu'on a de bonnes chances de le voir apparaître dans le 4.14.1 ou .2 dans les prochaines semaines, avant d'attendre la stabilisation du 4.15, sachant qu'un kernel panic doit être considéré comme un bug non négligeable. :)

      • [^] # Re: Mauvais timing...

        Posté par . Évalué à 5 (+3/-0).

        Oui, tout à fait. Je me suis mal exprimé mais en disant "stable", je voulais dire la prochaine sortie corrective (c-à-d mineur) des noyaux stables.

  • # test du correctif

    Posté par . Évalué à 10 (+16/-0). Dernière modification le 12/11/17 à 22:42.

    On fait donc un patch de la ligne err = -EFAULT; pour la changer en return -EFAULT;

    Tu n'en parles pas, mais je suppose que tu as testé ton correctif avant de proposer le patch, n'est-ce pas ?

    ("tester" signifiant ici : recompiler le noyau, booter dessus, et constater la correction)

  • # Et donc, comment se passe ton boot, maintenant ?

    Posté par . Évalué à 10 (+9/-0).

    Merci pour cette explication très claire.

    Ma question est dans le titre : comment se passe ton boot, maintenant que tu gères mieux l'erreur (mais il y a toujours l'erreur, non ?) ?

  • # git ?

    Posté par . Évalué à 2 (+0/-0).

    Aurait-il été possible d'éviter l'étape GDB en regardant les logs de l'arborescence concernée ? Je n'ai pas vérifié, je suis peu doué avec git

    Membre de l'april, et vous ? http://www.april.org/adherer

    • [^] # Re: git ?

      Posté par . Évalué à 4 (+2/-0). Dernière modification le 16/11/17 à 16:52.

      Apparemment pas :

      git blame ecc.c

      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  963)       * This condition is met by the default RNG because it selects a favored
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  964)       * DRBG with a security strength of 256.
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  965)       */
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  966)      if (crypto_get_default_rng())
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  967)              err = -EFAULT;
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  968) 
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  969)      err = crypto_rng_get_bytes(crypto_default_rng, (u8 *)priv, nbytes);
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  970)      crypto_put_default_rng();
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  971)      if (err)
      6755fd269d5c1 (Tudor-Dan Ambarus   2017-05-30 17:52:48 +0300  972)              return err;
      

      … et :

      git log --format='%h %an %ad %s' ecc.c

      6755fd269d5c Tudor-Dan Ambarus Tue May 30 17:52:48 2017 +0300 crypto: ecdh - add privkey generation support
      7380c56d2fca Tudor-Dan Ambarus Tue May 30 15:37:56 2017 +0300 crypto: ecc - rename ecdh_make_pub_key()
      ad269597107e Tudor-Dan Ambarus Thu May 25 10:18:05 2017 +0300 crypto: ecc - remove unnecessary casts
      099054d735a5 Tudor-Dan Ambarus Thu May 25 10:18:04 2017 +0300 crypto: ecc - remove unused function arguments
      8f44df154de7 Stephen Rothwell Fri Jun 24 16:20:22 2016 +1000 crypto: ecdh - make ecdh_shared_secret unique
      3c4b23901a0c Salvatore Benedetto Wed Jun 22 17:49:15 2016 +0100 crypto: ecdh - Add ECDH software support
      (END)
      

      Donc, le fichier incriminé a été créé en 2016 et n'a fait l'objet que de six patches en tout, donc les quatre derniers au printemps dernier. La ligne fautive se trouve dans le tout dernier patch, qui lui même ne modifie pas l'existant mais déclare de nouvelles fonctions, qui n'ont donc encore jamais été amendées jusqu'ici.

      C'est donc un authentique bug qui a passé tous les filtres jusqu'à présent.

  • # Questions

    Posté par . Évalué à 9 (+9/-0).

    Merci pour cet article. 2 petites questions :
    - qu'entends-tu exactement par "ma carte mère n'a pas le nécessaire pour sauvegarder un kernel panic hélas" ?
    - par curiosité, quel est l'outil ou le composant matériel qui génère une clé au démarrage ?

Envoyer un commentaire

Suivre le flux des commentaires

Note : les commentaires appartiennent à ceux qui les ont postés. Nous n'en sommes pas responsables.