diff --git a/code/test/getint.c b/code/test/getint.c index f1cc180..c6c2507 100644 --- a/code/test/getint.c +++ b/code/test/getint.c @@ -5,6 +5,5 @@ int main(){ int number; GetInt(&number); PutInt(number); - PutChar('\n'); } while(1 == 1); } diff --git a/code/test/getstring.c b/code/test/getstring.c index 1cb6a89..47ed2ad 100644 --- a/code/test/getstring.c +++ b/code/test/getstring.c @@ -1,9 +1,15 @@ #include "syscall.h" -#define MAX_INPUT_SIZE 4 +#define MAX_INPUT_SIZE 16 int main(){ char t[100]; + // this test write write on the output what the user entered in the input + // If we remove the loop, all caracters after MAX_INPUT_SIZE will be writed + // on my linux shell... A quite big security flaw, imagine + // 1234567812345678sudo rm -rf /\n ;) do { + //in our loop, char before the MAX_INPUT_SIZE sill be passed to the + // next getstring... GetString(t, MAX_INPUT_SIZE); PutString(t); PutChar('\n'); diff --git a/code/userprog/exception.cc b/code/userprog/exception.cc index 948f6a7..9177801 100644 --- a/code/userprog/exception.cc +++ b/code/userprog/exception.cc @@ -68,17 +68,23 @@ int copyStringFromMachine(int from, char *to, unsigned size) unsigned i = 0; int res; - // Need to read size-1 to put final /0 if needed - while((i < size) && (machine->ReadMem(from+i,1,&res))){ + // Need to read `size` value into memory to put final /0 if needed + for(i=0; i < size; i++){ + + // What should we do if a problem occurs when accessing + // memory? + if (machine->ReadMem(from+i,1,&res) == false){ + DEBUG('s', "Error When reading memory in copyStringFromMachine\n"); + return -1; + } DEBUG('s', "\n ->copyFromMachine keycode:%d", res); - *(to+i) = (char)res; - if ((char)res == '\0'){ - return i; - } - i++; + *(to+i) = (char)res; + if ((char)res == '\0'){ + return i; + } } - *(to+i)='\0'; - return size; + *(to+size)='\0'; + return i; } int copyStringToMachine(int to, char* from, unsigned size) @@ -89,7 +95,10 @@ int copyStringToMachine(int to, char* from, unsigned size) if (from[i] == '\0') { break; } - machine->WriteMem(to+i,1,(int)from[i]); + if ( machine->WriteMem(to+i,1,(int)from[i]) == false) { + DEBUG('s', "Error when writing memory in copyStringFromMachine\n"); + return -1; + } } // Write the last /0 @@ -148,7 +157,7 @@ ExceptionHandler (ExceptionType which) case SC_GetChar: { DEBUG ('s', "GetChar.\n"); - // Get the char input an write it to the register n.2 + // Get the char input and write it to the register n.2 int c = consoledriver->GetChar(); machine->WriteRegister(2, c); break; @@ -165,9 +174,11 @@ ExceptionHandler (ExceptionType which) // Do not process char before size // but for now we don't have any method to determine if // string passed to our function est bigger than expected (size) + // It seems to be easier to check than in userspace mode int n_char = ( size < MAX_STRING_SIZE) ? size : MAX_STRING_SIZE; - // allocate on byte more than buffer to put our \0 if needed + // allocate on byte more than buffer to put our \0 at the end, + // if needed char *write = new char[n_char + 1]; consoledriver->GetString(write, n_char); writesize = copyStringToMachine(addr, write, n_char); diff --git a/rapports/td1/rapport.md b/rapports/td1/rapport.md index 06bfaaa..cc05f82 100644 --- a/rapports/td1/rapport.md +++ b/rapports/td1/rapport.md @@ -1,5 +1,5 @@ --- -title: systèmes d'exploitation, rapport TD1 +title: systèmes d'exploitation, TD1 documentclass: scrartcl author: - Cédric Cassin @@ -15,16 +15,15 @@ linkstyle: bold ## Bilan -Ce premier TD est notre premier contact avec Nachos, et il faut bien avouer -qu'il ne fut pas aussi mélodieux que dans *Rencontre du Troisième Type* de Steven -Spielberg. +Ce premier TD est notre première approche de Nachos, et il faut bien avouer +qu'il ne fut pas aussi mélodieux que le premier contact dans *Rencontre du +Troisième Type* de Steven Spielberg. -Pour nous qui sommes sortis de la licence ADSILLH [^n_adsillh] nous -avions un avantage sur nos autres camarades: nous avons eu des **cours (et TDs) sur -les appels systèmes GNU/Linux**. Nous avons aussi un handicap : nous sommes -inscrits en Master IDI, ce qui signifie que nous avons une vie professionnelle -en plus de notre vie étudiante, et pour l'un de nous une vie familiale. En gros -mois de temps personnels à consacrer à Nachos. +Nous qui sommes sortis de la licence ADSILLH [^n_adsillh] disposons un avantage +sur certains de nos autres camarades: nous avons eu des **cours (et TDs) sur les +appels systèmes GNU/Linux**. Nous avons aussi un handicap : nous sommes inscrits +en Master IDI, ce qui signifie que nous avons une vie professionnelle en plus de +notre vie étudiante. En gros moins de temps personnels à consacrer à Nachos. Nachos n'en reste pas mois un sujets intéressant -- bon effectivement nous n'en ferons pas notre dernière vague tel Patrick Swayze dans *Point Break* -- mais @@ -36,34 +35,41 @@ developer ces premiers appels systèmes fut un travail enrichissant. ### Ce qui manque Comme vous pouvez l'imaginer après avoir lus les lignes précédentes, nous -n'avons pas eu le temps d'implémenter `printf` au moment où nous écrivons ces -lignes[^n_finir]. +n'avons pas eu le temps d'implémenter un élément : `printf` manque à l'appel au +moment où nous écrivons ces lignes[^n_finir]. [^n_finir]:la première version de ce rapport mentionnait aussi `PutInt` et `GetInt` mais nous avons réussi à les ajouter à la dernière minute. +Nous avons aussi implémenté la gestion des erreurs de mémoire dans les fonctions +`copyStringFromMachine` et `copyStringToMachine`, ces fonctions retourne **-1** +si une erreur se produit dans les instructions `ReadMem` et `WriteMem` (elles +retourne alors *false*), **mais** nous n'avons pas implémenté la gestion de ce +cas de figure dans les appels `PutString` et `Getstring`. + ### Ce que nous avons -Nous avons cependant bien implémenté `PutChar`, `PutString`, `GetChar` et -`Getstring`. Nous somme globalement satisfait du code que nous avons produit. -Cependant des doutes subsistent sur l'implémentation de `Getstring`. +Nous avons implémenté `PutChar`, `PutString`, `GetChar` et `Getstring`. +Nous sommes globalement satisfait du code que nous avons produit. Cependant des +doutes subsistent sur l'implémentation de `Getstring`. Nous avons aussi écrit les fonctions `CopyStringToMachine` et `CopyStringFromMachine` que nous avons positionné dans le fichier `userprog/exception.cc`. Ces deux fonctions pourraient en effet servir pour -d'autres appels systèmes. +d'autres appels système. ### Les bugs A part un bug lors de la compilation de notre source de test pour l'appel système `PutString`, nous n'avons pas repérés de bugs sur nos différents ajouts. -Ce qui ne signifie pas pour autant qu'il n'y en a pas, mais les différents tests -mis en place n'ont pas révélés de problèmes particuliers. +Ce qui ne signifie pas pour autant qu'il n'y en ai pas, mais les différents +tests mis en place n'ont pas révélés de problèmes particuliers. Le problème avec notre test de `PutString` (le source est dans -`test/pustring.c`) se produit à la compilation. Il semblerait aue si la chaine -de caractère passées en paramètre est inférieure à la taille du tampon -(`MAX_STRING_SIZE`), alors l'erreur suivante apparaît à la compilation: +`test/pustring.c`) se produisait à la compilation. d'après nos constatation, il +semblerait que si la chaine de caractère passées en paramètre est inférieure à +la taille du tampon (`MAX_STRING_SIZE`), alors l'erreur suivante apparaît à la +compilation: > putstring.c:(.text+0x10): dangerous relocation: Réadressage relatif GP utilisé alors que GP n'est pas défini @@ -86,9 +92,10 @@ dans *Les Goonies*. [^n_modifs]:Pour les éléments où nous n'étions pas guidés par le sujet du TD. Le point le plus délicat fut tout de même l'écriture de l'appel système -`GetString`. Elle a nécessite beaucoup d'aller retour **compilation / test / +`GetString`. Il a nécessite beaucoup d'aller retour **compilation / test / modification** un peu à la manière d'un jeu-vidéo de type Rogue-Like (aucune -manette de jeu, clavier ou souris a été blessée ou même malmenée durant ce TD). +manette de jeu, clavier ou souris a été blessé ou même malmené durant ce +TD...). Afin de progresser sur cette partie, nous avons ajouté des `DEBUG` dans notre code. Ainsi nous affichons **tout un tas d'informations utiles**. @@ -104,16 +111,36 @@ avons réussi à obliger `GetString` à respecter le paramètre `size` en compta le nombre de caractères écrits, nos **n'avons pas réussi à détecter ce cas de figure** pour éventuellement lancer une exception. +Un problème de sécurite peux alors se produite et il est simple à tester avec +`test/getstring_2`, le voici illustré : + +``` +/test@machine $ ./nachos -x ../test/getstring_2 +12345678912345678ls +1234567891234567 +[...] +test@machine $ ls +addrspace.cc bitmap.cc consoledriver.cc console.o +[...] +``` + +On voit bien que les caractères dépassant le tampon de 16 caractères on **été +transmit à la console du système hôte** de NachOS. dans notre test, un simple +`ls` a été effectué, mais un `rm -rf ~` serait un peu plus problematique. + +Nos différents tests sur le chaînes de caractères mettent aussi en lumière une +limitation de NachOS : **Il n'est pas très doué question caractères accentués**, +et a l'air de ne pas parler l'UTF-8 mais cette limitation n'est pas de notre +fait. + ## Les tests Pour `PutString`, nous avons écrit un petit programme permettant de tester -quelques sorties plus ou moins longue. Les éléments de debug nous ont permis de + corriger un problème de caractère qui passait à la trappe lors du passage au tampon suivant. -Le test mets aussi en lumière une limitation de NachOS : **Il n'est pas très -doué question caractères français**, mais elle n'est pas de notre fait. -Voici un exemple de sortie de notre appel avec la sortie de debug activée: +Voici un exemple de sortie de cet appel avec la sortie de debug activée: ``` [...]