La bibliothèque standard C n’était pas thread-safe, et même Rust sûr n’a pas empêché le crash
(edgedb.com)- Alors qu’EdgeDB déplaçait les E/S réseau de Python vers Rust, de nouveaux tests de HTTP fetch basés sur
reqwestsemblaient se bloquer uniquement sur la CI ARM64, mais il s’agissait en réalité d’un crash - L’analyse du core dump a montré que le point problématique n’était pas le nouveau code HTTP, mais l’intérieur de
getenv()danslibc: la fonction parcourait le tableau des variables d’environnement et a échoué en tentant de lire le pointeur invalide0x220 - Un autre thread définissait
SSL_CERT_FILEetSSL_CERT_DIRvia le cheminopenssl-probe, ce qui a amenésetenv()à réallouerenviron; en parallèle, le chemin de gestion d’erreur de Python appelaitgetenv(), créant une condition de concurrence - Le code Rust ne contenait pas d’
unsafeexplicite, mais pendant questd::env::set_var()modifiait l’environnement global, les autres runtimes ou les appels directs àlibcn’étaient pas synchronisés par le verrou interne de Rust - La solution a consisté, sous Linux, à passer à
rustlsau lieu du backendrust-native-tls/openssldereqwest; Rust 2024 edition et glibc évoluent aussi dans un sens qui réduit ce type de problème
Un crash qui ne se révélait que sur la CI ARM64
- EdgeDB était en train de porter une grande partie de son code d’E/S réseau de Python vers Rust, en développant une nouvelle fonctionnalité de HTTP fetch
- La bibliothèque client HTTP utilisée était
reqwest, et la fonctionnalité passait en local ainsi que sur les runners CI x86_64, mais échouait par intermittence sur les runners CI ARM64 - Au début, le runner de tests semblait se bloquer indéfiniment : dans les logs de CI, un test restait en cours d’exécution sans erreur jusqu’à expirer plusieurs heures plus tard
- La première hypothèse concernait une différence de modèle mémoire entre Intel et ARM64
- Intel a un modèle mémoire relativement strict, avec un ordre global des écritures mémoire sur lequel tous les processeurs s’accordent
- ARM a un modèle mémoire à ordre plus faible, où les écritures peuvent apparaître dans des ordres différents selon les threads
Suivre le core dump dans un environnement CI Docker
- La machine de CI nocturne tournait sur Amazon AWS, ce qui permettait de se connecter en tant que véritable utilisateur root hors du conteneur et de consulter
dmesgainsi que les journaux système - Le PID qui semblait bloqué a été recherché à l’intérieur et à l’extérieur du conteneur, mais le processus n’existait pas : cela a révélé qu’il s’agissait d’un crash, et non d’un deadlock
- Comme un conteneur Docker est un espace de noms de processus, le core dump a été transmis à l’hôte Docker, et
journalctla permis de confirmer un core dump du processuspython3 - À la première ouverture du core avec
gdb, la backtrace était inutilisable faute de fichiers.soprovenant de l’intérieur du conteneur - Après avoir copié
/lib,/usr, etc. depuis le conteneur et configurésolib-absolute-prefixdansgdb, le point de crash a été identifié commegetenv()danslibc.so.6
Le pointeur de variable d’environnement corrompu lu par getenv()
- La backtrace complète suivait le chemin
getenv()→__dcigettext()→strerror_r()→strerror()→PyErr_SetFromErrnoWithFilenameObjects() - Ce n’est pas le nouveau code HTTP qui a directement crashé : Python créait une exception fondée sur
errno, puis passait par un chemin lié à gettext qui appelaitgetenv() - L’implémentation de
getenv()dans GLIBC 2.17 parcourt l’environPOSIX comme une liste dechar **et inspecte les pointeurs vers les chaînes de variables d’environnement jusqu’au dernier pointeurNULL - D’après le désassemblage et l’état des registres,
getenv()a crashé en tentant de lire un octet à l’adressex19 = 0x2200x220était manifestement une adresse mémoire invalide- En inspectant
environlui-même, la liste courante des variables d’environnement paraissait cohérente
Cause : une condition de concurrence entre setenv() et getenv()
setenv()n’est pas une fonction sûre à appeler dans un environnement multithread, et les crashes étranges dansgetenv()delibcliés à ce problème ont été redécouverts à plusieurs reprises- En comparant le désassemblage avec le code C,
x20correspondait au pointeurepqui parcourt le tableauenviron - Au moment du crash,
x20valait0x248b5000, tandis que l’environcourant valait0x28655750, environ 60 Mo plus loin - En comparant la mémoire autour de l’ancien tableau d’environnement avec l’
environcourant, la dernière différence apparaissait sur les entréesSSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crtetSSL_CERT_DIR=/etc/ssl/certs - Il était possible d’en déduire qu’un autre thread avait déplacé
environpendant un appel àsetenv(), tandis quegetenv()continuait à lire l’ancien tableau d’environnement, une situation de use-after-free
Le lien avec openssl-probe et le backend TLS
- Un ancien ticket lié à
rust-native-tlsa fourni un indice :openssl-probedéfinit les variables d’environnementSSL_CERT_FILEetSSL_CERT_DIRpour trouver les certificats système - Sous Linux, ce chemin est appelé lorsque l’on utilise le backend
opensslderust-native-tls - Le code
openssl-probeen cause définit les deux variables d’environnement avecenv::set_var(), sansunsafeexpliciteSSL_CERT_FILESSL_CERT_DIR
- Cette combinaison a fait qu’un code Rust sans unsafe interagissait mal avec l’usage de
libcdans le même processus, provoquant un crash
Pourquoi cela s’est reproduit sur ARM64 Linux
- Ce crash se produit lorsque
setenv()déplace le tableau d’environnement viareallocau moment précis où un autre thread appellegetenv() - Plusieurs conditions devaient être réunies pour le reproduire
- Le nombre de variables d’environnement devait être tel qu’il déclenche un
realloc - Un échec d’E/S sans lien direct devait être capturé par
asyncio - Le chemin de gestion d’erreur de Python devait appeler
getenv()exactement au même moment pour obtenir la variable d’environnementLANGUAGE
- Le nombre de variables d’environnement devait être tel qu’il déclenche un
- La valeur erronée
0x220était proche de l’ancienne taille de l’environnement en mots de 64 bits0x220 / 8 = 68- Cette valeur avait écrasé l’emplacement du
NULLfinal de l’ancien bloc d’environnement, et semblait être une valeur utilisée par le malloc système pour indiquer la taille d’un bloc libéré
- Comme de nombreuses conditions préalables étaient nécessaires, le fait même que le problème soit assez reproductible sur une seule plateforme relevait de la chance
Les indices observés dans le désassemblage ARM64
- Une confusion venait du fait que, dans le désassemblage de
getenv(), le point oùx20changeait n’était pas évident - La clé était le mode d’adressage pré-indexé d’AArch64
ldr x19, [x20, #8]!fonctionne ainsix19 = *(x20 + 8)x20 = x20 + 8
- À cause de ce mode d’adressage,
x20parcourait bien le tableau de pointeurs des variables d’environnement, même s’il n’était pas explicitement écrit à gauche
Correctif appliqué et évolutions des projets concernés
- La décision finale a été de migrer, sous Linux, vers
rustlsau lieu du backendrust-native-tls/openssldereqwest - Le choix initial du backend TLS natif visait à éviter d’embarquer deux moteurs TLS pendant le portage du code Python vers Rust
- Après ce problème, embarquer temporairement deux moteurs TLS a été jugé acceptable
- Une autre solution aurait consisté à effectuer le premier appel à
try_init_ssl_cert_env_vars()en tenant le GIL de Python- Rust possède un verrou interne qui empêche les courses entre morceaux de code Rust lisant et écrivant l’environnement
- Mais cela ne bloque pas les cas où du code dans d’autres langages utilise directement
libc - Tenir le GIL permettrait au moins d’éviter la concurrence avec les threads Python
- Le projet Rust avait déjà identifié ce problème et prévoit de rendre les fonctions de modification de l’environnement
unsafedans l’édition 2024 - Le projet glibc a aussi récemment ajouté une modification qui améliore la thread-safety de
getenv()en évitantreallocet en laissant fuir les anciens tableaux d’environnement
4 commentaires
setenvn’est pas thread-safe, et le C ne veut pas corriger celaLa fonction
setenvfait encore des siennes.J’écrirais le titre ainsi : « L’absence de sûreté des threads dans la stdlib C, un échec que même Rust, pourtant sûr, ne peut pas sauver ». :)
J’ai bien compris avec certitude.
Avis de Hacker News
Le point le plus important ici, c’est que dans la prochaine édition de Rust, les fonctions de définition des variables d’environnement deviendront unsafe
Avec un peu de chance, cela aura aussi des répercussions sur les crates qui provoquent ce type de crash ; entre-temps, une issue a été ouverte upstream : https://github.com/alexcrichton/openssl-probe/issues/30
getenvetsetenvouunsetenven toute sécurité depuis des threads différents, n’est pas réellement corrigéLa seule solution fiable semble être de modifier ces fonctions pour qu’elles prennent obligatoirement un mutex
Malheureusement, l’écosystème n’en est pas là : https://github.com/seanmonstar/reqwest/blob/master/Cargo.tom...
____UNSAFE_payattention__nevermindthatthisappears50timesinthisfile___C’est similaire dans les frameworks web : Vue a la directive
v-htmlet React adangerouslySetInnerHTML; sur ce point, je pense que Vue est clairement meilleurset_varetremove_varde la bibliothèque standard Rust exigeront correctement un blocunsafe {}dans la prochaine édition, l’édition 2024La documentation mentionne désormais aussi les problèmes de sûreté, mais rendre ces fonctions safe dès le départ était une erreur, une erreur que d’autres langages de plus haut niveau ont également commise
https://doc.rust-lang.org/stable/std/env/fn.set_var.html
glibc dispose d’un patch qui rend
getenvsûr dans davantage de cas où l’environnement est modifié, mais en C on peut toujours accéder directement àenviron, donc cela ne pourra jamais être totalement sûr dans les situations où des modifications se produisent : https://github.com/bminor/glibc/commit/7a61e7f557a97ab597d6f...Il en résulte une fuite mémoire de taille constante amortie pour chaque variable d’environnement active, mais les variables elles-mêmes présentent déjà ce type de fuite, en plus dépendante de leur longueur, et cela inclut même des valeurs qui ne sont plus utilisées
Il doit certainement exister des cas d’usage pathologiques où, même dans des programmes corrects vis-à-vis de l’API, la mémoire augmente indéfiniment à cause de cela
C’est intéressant, mais un peu dérangeant : pour corriger des programmes qui enfreignent les règles en utilisant l’API depuis plusieurs threads, on semble introduire dans les programmes qui respectent l’API un bug de croissance mémoire illimitée. Cela donne une impression de pragmatisme plutôt que de dogmatisme
unsafeMême si les mainteneurs de la bibliothèque standard C s’opposent à rendre
setenvsûr en multithread, il faudrait au minimum définir une nouvelle API thread-safe, que ce soit dans POSIX ou en créant d’abord un standard de fait que POSIX adopterait ensuiteSi le temps passé à expliquer pourquoi on ne peut rien faire avait été consacré à corriger ce problème, il aurait été possible de remplacer l’ancien
setenvet de le déprécier puis de le supprimer dans de nombreux projets logicielsLe fait que glibc apporte des changements visant à faire quasiment disparaître ce problème rend peu convaincante l’affirmation du mainteneur de Musl selon laquelle il serait impossible de le corriger dans Musl
extern char **environ;Tant que
environest publiquement accessible, rien ne garantit même quesetenvetgetenvsoient utilisés, car ils ne sont pas obligatoiresSi l’on pouvait supprimer
environ, rendresetenvetgetenvthread-safe serait assez simple. Si l’on ne peut pas le supprimer, c’est impossible ; on peut toutefois soutenir que rendresetenvetgetenvthread-safe resterait une amélioration, même si ce n’est pas une solution complèteexec()qui ne prennent pas l’environnement en argument ou qui recherchent dansPATHpour trouver l’exécutable devront probablement aussi prendre un verrouSous Linux, tomber sur des bugs liés aux variables d’environnement ressemble à une sorte de rite de passage, alors que sur les autres Unix, curieusement, c’est généralement moins problématique.
Linus et le noyau corrigent les bugs POSIX de façon pragmatique, en faisant en sorte qu’ils ne se manifestent pas vraiment, tandis que glibc reste à la traîne alors que cela fait des décennies que des gens essaient d’atténuer ne serait-ce qu’un peu le problème, ce qui est assez amusant.
Il est vrai qu’il existe toutes sortes de casse-têtes comme
TZ, mais sigetenv_r()avait été fourni, synchronisé avecsetenv(), et si l’utilisation degetenv()avait simplement déclenché un avertissement à la compilation et à l’édition de liens, une grande partie des problèmes aurait disparu.On aurait même pu aller plus loin avec une approche copie à l’écriture (COW) où les pointeurs d’environnement restent en lecture seule.
À la place, le problème a été refilé à chaque application, ce qui est une grosse erreur, car les auteurs d’applications peuvent difficilement savoir ce que font leurs dépendances. C’était aussi la situation dans laquelle je me suis retrouvé il y a longtemps, et à l’époque le fournisseur de la bibliothèque propriétaire m’avait dit d’arrêter d’utiliser ce clone Unix jouet qu’était Linux.
Le problème n’est pas l’implémentation, mais l’API elle-même.
setenv(),unsetenv(),putenv(), et surtoutenviron, sont intrinsèquement dangereux dans les programmes multithread.Même
getenv_r()ne peut pas tout sauver. Pendant qu’un thread copie l’ancienne valeur d’une variable d’environnement dans le tampon fourni, un autre thread peut appelersetenv().Bien sûr,
getenv_r()corrige le cas où, après avoir reçu une valeur viagetenv(), un autre thread appellesetenv()et invalide cette mémoire, mais il n’existe aucun moyen d’empêcher les autres appels qui cassent l’API.La libc peut atténuer une partie du problème en prenant un mutex dans
getenv()/setenv()/putenv()/unsetenv(), mais elle n’a toujours aucun moyen de garantir que la valeur renvoyée pargetenv()restera valide assez longtemps pour que le code appelant l’utilise.Il n’y a pas non plus de bonne façon de rendre sûr l’accès direct à
environ. On pourrait rendreenvironlocal à chaque thread, mais alors la vision de l’environnement pourrait diverger définitivement d’un thread à l’autre, et les résultats d’un appel àgetenv_r()pourraient différer de ceux d’une consultation directe deenviron.Ici, préserver la compatibilité descendante est vraiment difficile, et le simple fait d’ajouter un mutex pour protéger ces fonctions pourrait déjà modifier la sémantique de programmes existants et les casser.
Il y avait déjà eu un article disant que
setenvétait épouvantable : https://www.evanjones.ca/setenv-is-not-thread-safe.htmlIl y avait aussi eu une discussion, et dès le premier commentaire il était question de problèmes causés en Rust : https://news.ycombinator.com/item?id=38342642
Ici, la plupart des autres problèmes semblent relever de l’environnement de développement. Les tests étaient exécutés avec Docker sur une machine distante dans un datacenter Amazon, et cette machine n’arrivait pas à signaler le crash du processus.
En plus, il n’y avait pas assez d’informations de symboles de débogage dans le conteneur pour obtenir une trace de pile. Si une trace propre avait été obtenue dès le premier échec, cela aurait été immédiatement évident.
Je me demande surtout pourquoi utiliser
setenvau départ.Cette discussion me rappelle le mouvement 12-factor app, auquel plusieurs anciens collègues croyaient énormément. L’un de ces « factors » était que la configuration de l’application devait se faire via des variables d’environnement
J’ai toujours trouvé ça un peu idiot, parce que ce mode de configuration revient à mettre des valeurs de type chaîne dans un espace de noms plat, comme dans un panier
Je vois aussi les dangers de
getenv()/setenv()/environcomme un argument fort pour ne pas utiliser les variables d’environnement pour la configurationBien sûr, il n’existe pas toujours une excellente alternative bien prise en charge. Je préfère les fichiers de configuration, et on peut aussi utiliser des configurations modèles où l’on ne renseigne que les valeurs de développement, de staging et de production. En général, malgré ses défauts et ses pièges, j’utilise YAML ; il existe peut-être de meilleurs formats de fichier de configuration, mais je considère que YAML est nettement préférable aux variables d’environnement
Dans NT, les variables d’environnement peuvent être typées et modélisées, et il y a aussi le registre, une base de données de configuration avec espaces de noms. C’est verbeux et étrange, certes
En plus, MSVC fournit des versions thread-safe de presque toutes les fonctions de la bibliothèque standard
J’entends souvent de nouveaux développeurs C/C++ déplorer le manque de compatibilité POSIX de MSVC, mais ils ne semblent pas réfléchir très profondément à ce que cela veut dire en pratique. Cela ressemble plutôt au souhait d’être compatible avec des programmes C écrits dans les années 1990
Cela nuit à la capacité de déduire le comportement à partir de la seule signature d’une fonction, et rend impures beaucoup de fonctions qui auraient pu être pures
S’il existait une fonctionnalité de langage permettant de marquer une application pour que, dans n’importe quel processus, les variables d’environnement ne puissent pas être utilisées et ne puissent être lues qu’une seule fois, en une seule opération globale et non variable par variable, je pense que je l’utiliserais partout
getenv()lui-même ne pose aucun problème ; le problème, c’estsetenv()En théorie, l’environnement est défini avant le lancement de cette mystérieuse application, donc on ne devrait pas avoir besoin de l’utiliser
Mais un espace de noms plat, des valeurs sous forme de chaînes, et un espace global libre et partagé par tout le monde sans savoir quelles bibliothèques ni quels modules s’y invitent, ce n’est pas une bonne idée même sans les problèmes de sûreté de
setenv()La plupart des problèmes évoqués ici semblent venir de l’abus consistant à vouloir utiliser l’environnement comme un stockage clé-valeur pour de l’état global mutable. Je ne vois pas pourquoi on voudrait faire ça
La JVM traite en pratique l’environnement comme immuable, et il est possible que des entreprises utilisant Scala et Java, comme SoundCloud, aient influencé le mouvement 12-factor app. Je n’ai jamais rencontré de cas où l’environnement changeait ou provoquait des problèmes de threading
Même si l’environnement change, la copie immuable créée au démarrage de la JVM reste telle quelle, et le code qui interagit avec l’environnement via les API Java ordinaires ne voit pas cette modification
Le problème des fichiers de configuration, c’est que le parsing est propre à chaque processus. C’est pour cela que Linux/Unix est si désordonné. Chaque outil a ses propres conventions et mécanismes de configuration, sans standard
Dans l’écosystème Docker, peu importe ce que l’on fait à l’intérieur du conteneur : l’interface avec l’extérieur consiste soit à monter des volumes et à suivre le système de configuration complexe de chaque application, soit tout simplement à utiliser des variables d’environnement
Aujourd’hui, la plupart des logiciels modernes exécutés avec Docker sont assez compatibles avec Docker pour que leur comportement puisse être entièrement contrôlé par l’environnement, et dans beaucoup de cas c’est suffisant
Avec Docker Compose ou Kubernetes, on se retrouve avec une liste de variables d’environnement définissant le lancement du processus dans un fichier YAML, ce qui donne dans une certaine mesure la structure souhaitée. Je n’aime pas YAML, mais ça fonctionne suffisamment bien ; les problèmes de syntaxe ont de bonnes chances de gâcher une journée, mais les alternatives ne sont pas exemptes de problèmes non plus
Moi aussi, j’utilise le style 12-factor app, mais une fois les variables d’environnement et les données entrées dans l’application, je les valide et les stocke. Après ça, il n’y a plus aucun problème
C’est un excellent article qui creuse un bug difficile à repérer
Il y avait de tout : bug intermittent, spécificité d’architecture, bug caché dans une dépendance, Rust, le GIL de Python, et même gettext
Ces rapports de résolution de problème détaillés sont ce qui se rapproche le plus du fait de l’avoir vécu soi-même. Dans une situation où une dépendance utilise ça et où l’on ne pouvait pas le savoir, il est difficile de dire facilement « il suffisait de ne pas utiliser X »
Ils disent que « les machines de CI nocturne tournent sur Amazon AWS et ont l’avantage de pouvoir utiliser un vrai utilisateur root, pas un conteneur », tout en disant aussi que « les fichiers nécessaires n’existent pas hors du conteneur, et le conteneur est tellement minimal qu’il n’est pas facile d’y installer
gdb»Les gens ont-ils désormais perdu la capacité de compiler et déboguer en local, sans cloud ni conteneurs ?
Pour faire des choses très triviales, il faut désormais toutes sortes de complexités cloud et de couches de déploiement. On a inversé à 100 % la révolution du PC pour revenir à l’époque lourde et coûteuse du mainframe computing
La raison, c’est que l’argent est dans le cloud, et que le cloud est un DRM. Si vous y mettez le logiciel, vous pouvez facturer un abonnement, il est impossible de le contourner, et vous conservez indéfiniment une dépendance totale. Souvent, les utilisateurs ne peuvent même pas récupérer leurs propres données
Cela permet aussi de faire facilement de l’analytique en temps réel pour optimiser le produit
L’architecture informatique est en aval du modèle économique. Si le mainframe a d’abord disparu, c’est parce qu’il n’y avait pas Internet et que les PC étaient moins chers, mais aussi parce que les fournisseurs ont perdu une grande partie de leur pouvoir d’enfermement
Désormais, il existe un moyen de ressusciter un modèle bien plus rentable. La liberté gênante des utilisateurs a disparu, et franchement, quand les utilisateurs obtiennent ce genre de liberté, ils finissent souvent par ne pas payer, ce qui rend difficile l’existence d’entreprises de logiciels de qualité
Il est très probable qu’ils n’aient pas pu reproduire le crash en local. Les machines des développeurs étaient probablement majoritairement en x86, où le crash ne se produisait pas
Ils auraient dû mieux gérer le crash, mais ils semblent eux-mêmes conscients du problème, et ce n’est pas le point central traité ici
L’état global mutable, c’est le mal. Entre amis, on ne laisse pas ses amis utiliser de l’état global mutable
Je déteste les variables d’environnement. C’est la « façon Linux » de faire, mais je les évite comme la peste. Je recommande vivement
libc est épouvantable, et le monde devrait passer à autre chose
Le problème, ce n’est ni Linux, ni l’état global mutable ou les ressources, ni libc
Le problème, c’est qu’au travail on ne nous donne pas le temps de faire les choses correctement. Par exemple, pour repérer le problème avec GDB avant qu’il n’éclate, il faut que votre supérieur vous laisse le temps de déboguer obstinément le code et tout ce qu’il touche, puis de remonter la piste
Il y a trop d’argent investi dans du code à moitié cuit. C’est triste, mais vrai
Il y a tellement de problèmes de ce genre qu’au final j’ai patché
getenv/setenv/putenvavecLD_PRELOAD