search

polkit-github-migration-bot / t4_polkit

Other
0 stars 0 forks source link

SpiderMonkey APIs used incorrectly #99

Open polkit-github-migration-bot opened 4 years ago

polkit-github-migration-bot commented 4 years ago

In gitlab.freedesktop.org by ptomato on Nov 28, 2019, 24:51

Link to the original issue: https://gitlab.freedesktop.org/polkit/polkit/-/issues/97 Similar to #88 but more expansive, there are a number of misuses of the SpiderMonkey API in the JS backend. Functions like JS_SetProperty() must have their return value checked before making another SpiderMonkey API call, because they can cause an exception to be thrown within the JS engine. Any pending JS exception should be logged and cleared at the end of each JS::Evaluate() or JS_ExecuteScript() or JS::CallFunctionName() call.

So, for example, set_property_str() and all the similar functions need to return bool and whatever calls them needs to bail out on failure.

Logging the JS exception will have the added benefit of giving more useful log output when a rule fails to execute, rather than just "Error evaluating authorization rules."

In addition we have near the top of the file

#ifdef JSGC_USE_EXACT_ROOTING
/* See https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/GC/Exact_Stack_Rooting
 * for more information about exact stack rooting.
 */
#error "This code is not safe in SpiderMonkey exact stack rooting configurations"
#endif

If this error message is true, then this is probably also a problem... Exact stack rooting is the only possible configuration now, so much so that they have stopped bothering to define the JSGC_USE_EXACT_ROOTING macro :smile:. However, the only thing that I can see being a problem at first glance is that priv->js_global and priv->js_polkit must be traced during garbage collection, because the garbage collector might move them and leave the pointers dangling. This should be done in a callback added with JS_AddExtraGCRootsTracer().

polkit-github-migration-bot commented 3 years ago

In gitlab.freedesktop.org by Fat-Zer on Jun 20, 2021, 14:20

That's one of a few results found by Error evaluating authorization rules query, so I'm willing to share a story here in hope it could me helpful in any way to somebody.

Recently I've spent several wonderful hours debugging why none of queries to polkit work on my system. And all hints I had was the phrase Error evaluating authorization rules. I was pretty sure that the problem couldn't be about just about some specific rule: it was obvious that in such it wouldn't been affected unrelated interfaces, and it would output at least a file the rule was located at... I've tried a dozen of times to rebuild and reinstall , spidermonkey and its other various dependencies... I've tried to step through polkit execution to get just a bit more information why and how the hell it was happening... After all as a sign of absolute despair I've removed all the rules from both /usr/share/polkit-1/rules.d/ and /etc/polkit-1/rules.d/... And it just worked. It revealed that in fact it was caused by a single stale rule file, which were trying to call an already removed script (which I supposed just kept throwing an exception each time it couldn't find an executable).

I hope besides being helpful to some person struggling with the same problems, this story could be an illustration that the issue is actually important enough.