zhangqd / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
0 stars 1 forks source link

Infinite number of processes on Linux when cef_main_args_t (argc/argv) is filled with 0 and NULL #1199

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I forgot to set argc/argv members and it caused creation of 1000 processes in 
my system. The code to reproduce:

    cef_main_args_t mainArgs = {};
    cef_app_t app = {};
    cef_settings_t settings = {};
    settings.no_sandbox = 1;
    cef_initialize(&mainArgs, &settings, &app, NULL);

Original issue reported on code.google.com by czarek.t...@gmail.com on 11 Feb 2014 at 2:35

GoogleCodeExporter commented 9 years ago
As you know this is blatantly wrong API usage. You also forgot to call 
CefExecuteProcess. None of the sub-process launching will work correctly 
without command-line arguments. It's likely trying to create a zygote process 
but instead getting your main process, which then does it all over again. How 
would we protect against this?

Original comment by magreenb...@gmail.com on 11 Feb 2014 at 4:28

GoogleCodeExporter commented 9 years ago
Yes, this is wrong API usage. But it is crashing the whole system.

Even if you add a call to cef_execute_process, then still one million of 
processes is being created.

I can undestand application crashing because of wrong API usage. But crashing 
the whole system is a bit too much IMO.

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 4:45

GoogleCodeExporter commented 9 years ago
Let's compare this behavior to some other library. GTK for example also has a 
function that accepts args, gtk_init(argc, argv). If you pass 0, 0 to it then 
it still works fine. It doesn't crash my system.

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 4:50

GoogleCodeExporter commented 9 years ago
You didn't answer the question :) How would we protect against this?

Original comment by magreenb...@gmail.com on 11 Feb 2014 at 4:53

GoogleCodeExporter commented 9 years ago
In cef_initialize it should be checked if mainArgs.argc == 0, if so then return 
false immediately, do not execute any further code that creates one million 
processes :)

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 4:57

GoogleCodeExporter commented 9 years ago
Currently cef_initialize returns true(1). You pass wrong arguments, but it 
doesn't check it and thinks everything is OK. From docs:

   // A return value of true (1) indicates that it succeeded 
   // and false (0) indicates that it failed.

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 5:00

GoogleCodeExporter commented 9 years ago
Passing NULL/0 command-line arguments to CefInitialize is valid usage. Passing 
NULL/0 command-line arguments to CefExecuteProcess is arguably incorrect usage, 
but checking for that would not resolve the problem with the code in your 
original comment.

Original comment by magreenb...@gmail.com on 11 Feb 2014 at 3:00

GoogleCodeExporter commented 9 years ago
Why is it valid usage to pass 0/NULL to CefInitialize? What is the use case?

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 8:51

GoogleCodeExporter commented 9 years ago
@comment#8: Some frameworks don't provide access to argv/argc, so the 
application can pass NULL/0 and optionally implement 
CefApp::OnBeforeCommandLineProcessing.

Original comment by magreenb...@gmail.com on 11 Feb 2014 at 8:55

GoogleCodeExporter commented 9 years ago
Thanks for explaining. I've tested calling cef_execute_proces with valid args, 
and cef_initialize with 0/NULL and it works fine. So the problem with infinite 
processes happens only when cef_execute_process is not called.

Original comment by czarek.t...@gmail.com on 11 Feb 2014 at 8:57

GoogleCodeExporter commented 9 years ago
I think I've found a way to protect against this behavior. Just check if there 
was a call to cef_execute_process, if not then cef_initialize should return 
false. That's how cef2go users are protected from invalid CEF API usage:

https://github.com/CzarekTomczak/cef2go/commit/bf8e60d85b24ed088514d79afb68d5e22
9d4a558

Original comment by czarek.t...@gmail.com on 15 Feb 2014 at 4:09