Hi Patrik,
On 06.11.2012 11:51, Patrik Flykt wrote:
Hi,
On Fri, 2012-11-02 at 17:26 +0100, Daniel Wagner wrote:
From: Daniel Wagner <[email protected]>
Monitor changes on the config files. Either create, modify or destroy
them according the events we get from the inotify interface.
---
plugins/session_policy_ivi.c | 63 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 59 insertions(+), 4 deletions(-)
diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
index a34017a..5ac711f 100644
--- a/plugins/session_policy_ivi.c
+++ b/plugins/session_policy_ivi.c
@@ -25,6 +25,7 @@
#include <errno.h>
#include <string.h>
+#include <sys/inotify.h>
#include <glib.h>
@@ -35,6 +36,7 @@
#include <connman/log.h>
#include <connman/session.h>
#include <connman/dbus.h>
+#include <connman/inotify.h>
#define POLICYDIR STORAGEDIR "/session_policy_ivi"
@@ -174,10 +176,16 @@ static void selinux_context_reply(const unsigned char
*context, void *user_data,
goto done;
}
- policy = create_policy(ident);
- if (policy == NULL) {
- err = -ENOMEM;
- goto done;
+ policy = g_hash_table_lookup(policy_hash, ident);
+ if (policy != NULL) {
+ policy_ref(policy);
Isn't policy != NULL just an indication that the policy was found? Do we
need to ref the policy again?
There are two ways we can create a new policy. If a file exists, then
a policy object will be created. If later if a session is created
then this lookup should return that policy object. We need to add a ref
so that the object is not destroyed if the file is removed before the
session object is destroyed.
Yes, policy != NULL indicates there is already a policy object and we
need to grab a ref here so that it doesn't disappeared during the life
time of the session object.
+ policy->session = data->session;
+ } else {
+ policy = create_policy(ident);
What if we get an IN_DELETE and delete the original policy while looking
up a selinux context? Do we recreate it here again and why?
If a new file is added we lookup if a session already exists. If yes
we grab a reference on the policy object. Basically it is the opposite
use case from above.
File exist first:
- notify_handler creates policy object
- selinux_context_reply grabs ref on policy object
Session exists first:
- selinux_context_reply creates policy object
- notify_handler grabs ref on policy object
We do destroy the policy objects happens when neither a file nor a
session with given ident exists.
+ if (policy == NULL) {
+ err = -ENOMEM;
+ goto done;
+ }
}
g_hash_table_replace(session_hash, data->session, policy);
@@ -230,6 +238,7 @@ static void policy_ivi_destroy(struct connman_session
*session)
policy = g_hash_table_lookup(session_hash, session);
g_hash_table_remove(session_hash, session);
+ policy->session = NULL;
policy_unref(policy);
}
@@ -246,6 +255,44 @@ static int load_policy(struct policy_data *policy)
return 0;
}
+static void notify_handler(struct inotify_event *event,
+ const char *ident)
+{
+ struct policy_data *policy;
+
+ if (ident == NULL)
+ return;
+
+ if (event->mask & IN_CREATE) {
+ connman_info("Policy added for '%s'", ident);
+
+ policy = g_hash_table_lookup(policy_hash, ident);
+ if (policy != NULL)
+ policy_ref(policy);
I'm not following exactly how more than one create is generated for an
ident. Is the policy_ref() part ok here?
I hope I was able to explain it above. In short there are triggers which
can create a new policy object, it's either in selinux_context_reply()
or in notify_handler(). If an object already exists just grab a ref.
+ else
+ create_policy(ident);
+ }
+
+ if (event->mask & IN_MODIFY) {
+ connman_info("Policy modifed for '%s'", ident);
+
+ policy = g_hash_table_lookup(policy_hash, ident);
+ if (policy != NULL) {
+ load_policy(policy);
+
+ connman_session_config_update(policy->session);
+ }
+ }
+
+ if (event->mask & IN_DELETE) {
+ connman_info("Policy deleted for '%s'", ident);
+
+ policy = g_hash_table_lookup(policy_hash, ident);
+ if (policy != NULL)
+ policy_unref(policy);
+ }
Is the session allowed to exist after deleting its policy file? If not,
how many refs can we have for one policy?
Currently, if a session is created the default settings will be used. We
might want to allow to configure the default settings via main.conf or so.
To answer your questions, a session should be allowed to exists after
the policy file is deleted. But the default settings should be applied.
That bit is missing in this patch. So the session would be allowed to
keep going with the last settings. I am going to fix this.
Are we sure we don't have to
also do something like terminate an ongoing session or similar? The
question partly relates to the policy_ref() comment above.
I would just apply the default settings, that is what makes sense to me.
cheers,
daniel
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
