[MPlayer-dev-eng] [patch] cache2.c: when fork() fails, mplayer can kill every user process

[Yuriy Kaminskiy]

On 19.01.2010 23:55, Reimar Döffinger wrote:
> On Tue, Jan 19, 2010 at 06:26:19PM +0300, Yuriy Kaminskiy wrote:
>> Index: MPlayer/stream/cache2.c
>> ===================================================================
>> --- MPlayer.orig/stream/cache2.c	2010-01-19 17:56:38.000000000 +0300
>> +++ MPlayer/stream/cache2.c	2010-01-19 18:04:03.000000000 +0300
>> @@ -333,7 +333,17 @@ int stream_enable_cache(stream_t *stream
>>    }
>>  
>>  #if !defined(__MINGW32__) && !defined(PTHREAD_CACHE) && !defined(__OS2__)
>> -  if((stream->cache_pid=fork())){
>> +  {
>> +    pid_t pid = fork();
>> +    if (pid < 0) {
>> +      shmem_free(s->buffer,s->buffer_size);
>> +      shmem_free(stream->cache_data,sizeof(cache_vars_t));
>> +      stream->cache_data = NULL;
>> +      return 0;
>> +    }
>> +    stream->cache_pid=pid;
>> +  }
>> +  if (stream->cache_pid){
>>  #else
> 
> Freeing really doesn't belong here.
> You could test if something like the attached, completely untested patch works.

Don't have appropriate environment for testing, so, uhm.
Anyway, your patch clearly did not fixed this bug.

> Index: stream/cache2.c
> ===================================================================
> --- stream/cache2.c	(revision 30351)
> +++ stream/cache2.c	(working copy)
> @@ -351,6 +352,10 @@
>      }
>  #endif
>  #endif
> +    if (stream->cache_pid < 0) {

here we leave stream->cache_pid negative...

> +        mp_msg(MSGT_CACHE, MSGL_ERR, "Starting cache process/thread failed.\n");
> +        return 0;
> +    }
> @@ -284,13 +284,14 @@
>  
>  void cache_uninit(stream_t *s) {
>    cache_vars_t* c = s->cache_data;
> -  if(!s->cache_pid) return;
> +  if(s->cache_pid) {

... and here we happily kill(-1) exactly as before.

Slightly altered patch attached; compile-tested only.

> --- stream/stream.h	(revision 30351)
> +++ stream/stream.h	(working copy)
> @@ -114,7 +114,7 @@
>    off_t pos,start_pos,end_pos;
>    int eof;
>    int mode; //STREAM_READ or STREAM_WRITE
> -  unsigned int cache_pid;
> +  int cache_pid;

According to msdn _beginthread returns uintptr_t.

>    void* cache_data;
>    void* priv; // used for DVD, TV, RTSP etc
>    char* url;  // strdup() of filename/url

-------------- next part --------------
A non-text attachment was scrubbed...
Name: stream-cache-killall-3.patch
Type: text/x-diff
Size: 1484 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100120/98cc7615/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: stream-cache-killall-4.patch
Type: text/x-diff
Size: 967 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100120/98cc7615/attachment-0001.patch>