Re: [Mplayerplug-in-devel] fork() bug in mplayerplug-in ?

[Kevin D. <kde...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/14/10 08:13, James Pearson wrote:
> I've been trying to track down a problem we've been having with users 
> being logged out of their X session on Linux (CentOS5) - and it looks 
> like it's a problem with the mplayer plug-in in firefox - this is what I 
> believe is happening:
> 
> The return status of fork() in mypopen() is not being checked for failure.
> 
> If the fork() fails, then the return is -1 which results in 
> instance->pid being set to -1
> 
> This means that when killmplayer() is called later, it attempts to do:
> 
>   kill(-1, SIGTERM)
> 
> which kills _all_ the processes for which the user has permission to 
> kill - and logging the user out - which is not good.
> 
> We're still using mplayerplug-in 3.45 with firefox 3.0.X - but it looks 
> like the problem is still in the 3.55 code.
> 
> I believe the following patch should fix this issue:
> 
> --- ./Source/plugin-threads.cpp.dist    2008-06-13 04:38:37.000000000 +0100
> +++ ./Source/plugin-threads.cpp 2010-01-14 14:04:22.070539000 +0000
> @@ -196,7 +196,13 @@ FILE *mypopen(char **argv, pid_t * pid,
> 
>       child = fork();
> 
> -    if (!child) {
> +    if (child < 0) {
> +       if (DEBUG) {
> +               printf("Unable to fork\n");
> +       }
> +       return NULL;
> +
> +    } else if (!child) {
>          if (DEBUG) {
>              char **tmp;
>              tmp = argv;
> 
> Also, to prevent killmplayer() from ever trying to kill pid -1, then 
> maybe the following patch should also be applied:
> 
> --- ./Source/plugin-support.cpp.dist    2008-05-28 15:50:46.000000000 +0100
> +++ ./Source/plugin-support.cpp 2010-01-14 14:06:34.458130000 +0000
> @@ -561,7 +561,7 @@ void killmplayer(nsPluginInstance * inst
> 
>       instance->threadlaunched = 0;
> 
> -    if (instance->pid != 0) {
> +    if (instance->pid > 0) {
>          count = 0;
>          status = 1;
> 
> 
> James Pearson

James,

Patch has been applied to mplayerplug-in CVS.

Have you considered changing your users to gecko-mediaplayer/gnome-mplayer?

http://kdekorte.googlepages.com/gecko-mediaplayer

Kevin

- -- 
Get my public GnuPG key from
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x7D0BD5D1
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAktPNqcACgkQ6w2kMH0L1dH3WwCggKY0SatKSDiSup/daz2MhPZM
Ps0AoIGgWXY7tkwIYmyNkYPNGN1gd1El
=/M8H
-----END PGP SIGNATURE-----