Re: [tyndur-devel] [PATCH] libc: Fix fuer die Schreibpuffer in stdio

[Kevin Wolf <mail@xxxxxxxxxxxxx>]

Am Samstag, 25. April 2009 16:47 schrieb Antoine Kaufmann:
> ! libc: Fehler behoben, der verhindert, dass gepuffert wird, und zu
>         einem Buffer overflow führen kann.
> ! libc: Puffer in fclose und fseek flushen
> ! libc: Fix fuers Befuellen der Blockpuffer, damit dabei nichts
>         ueberschrieben wird.
> ---
>  src/modules/lib/stdlibc/file.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/modules/lib/stdlibc/file.c
> b/src/modules/lib/stdlibc/file.c index 34b8171..6441fcd 100644
> --- a/src/modules/lib/stdlibc/file.c
> +++ b/src/modules/lib/stdlibc/file.c
> @@ -201,6 +201,7 @@ int fclose (FILE* io_res)
>          return EOF;
>      }
>
> +    fflush(io_res);
>      setvbuf(io_res, NULL, _IONBF, 0);
>
>      result = rpc_get_dword(io_res->pid, "IO_CLOSE",
> sizeof(io_resource_id_t), @@ -489,10 +490,11 @@ size_t fwrite(const void*
> data, size_t blocksize, size_t blockcount, //Ueberpruefen, ob genug Platz
> im Buffer ist, dann werden die Daten // in den Buffer kopiert, sonst wird
> der Buffer geleert, und die // Daten werden direkt geschrieben.
> -			if (io_res->buffer_size < (io_res->buffer_pos + data_size))
> +			if (io_res->buffer_size >= (io_res->buffer_pos + data_size))
>  			{
>  				//Daten in den Buffer kopieren.
> -				memcpy(io_res->buffer_ptr, data, data_size);
> +				memcpy(io_res->buffer_ptr + io_res->buffer_pos, data,
> +                    data_size);

Tab-Damage. Aber schon im Original. :-/

>  				io_res->buffer_pos += data_size;
>                  return data_size;
>  			} else {
> @@ -508,7 +510,7 @@ size_t fwrite(const void* data, size_t blocksize,
> size_t blockcount, // geleert, und die Daten werden auch direkt
> geschrieben. Auch wenn // die Daten mindestens einen Zeilenumbruch
> enthalten, wird der // Buffer geleert.
> -			if ((io_res->buffer_size < (io_res->buffer_pos + data_size)) ||
> +			if ((io_res->buffer_size >= (io_res->buffer_pos + data_size)) ||
>  				(memchr(data, '\n', data_size) != NULL))

Ist die Logik hier nicht umgekehrt und die aktuelle Variante die richtige?

Wer auch immer diesen Code ursprünglich verbrochen hat, sollte sich 
schämen. ;-)