Re: fork() bug in mplayerplug-in ?

Kevin DeKorte <[email protected]> Thu, 14 Jan 2010 08:22:15 -0700
Newsgroups gmane.comp.mozilla.mplayerplug-in
Message-ID <[email protected]>
-----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-----

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev