Re: [PATCH 16/18] xenstored: use domain_is_unprivileged instead of checking conn->id

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote:
> This centralizes all the permission checking for privileged domains in
> preparation for allowing domains other than dom0 to be privileged.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  tools/xenstore/xenstored_core.c   |    6 +++---
>  tools/xenstore/xenstored_domain.c |    8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 4ec63f1..eea5fd6 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -488,7 +488,7 @@ static enum xs_perm_type perm_for_conn(struct connection *conn,
>  		mask &= ~XS_PERM_WRITE;
>  
>  	/* Owners and tools get it all... */
> -	if (!conn->id || perms[0].id == conn->id
> +	if (!domain_is_unprivileged(conn) || perms[0].id == conn->id

domain_is_unprivileged is:
        conn && conn->domain && conn->domain->domid != 0
        
which isn't quite the same as the code being replaced. The difference
appears to be the conn->id is valid for socket connections as well as
domain connections whereas conn->domain is only present for domain
connections.

Does this change not mean that, for the dom0-process xenstored
configuration we now treat socket based connections as unprivileged
where previously they would be unprivileged?


>                  || (conn->target && perms[0].id == conn->target->id))
>  		return (XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER) & mask;
>  
> @@ -826,11 +826,11 @@ static struct node *construct_node(struct connection *conn, const char *name)
>  	node->tdb = tdb_context(conn);
>  	node->name = talloc_strdup(node, name);
>  
> -	/* Inherit permissions, except domains own what they create */
> +	/* Inherit permissions, except unprivileged domains own what they create */
>  	node->num_perms = parent->num_perms;
>  	node->perms = talloc_memdup(node, parent->perms,
>  				    node->num_perms * sizeof(node->perms[0]));
> -	if (conn && conn->id)
> +	if (domain_is_unprivileged(conn))
>  		node->perms[0].id = conn->id;
>  
>  	/* No children, no data */
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index 648eb1d..5f4a09e 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -336,7 +336,7 @@ void do_introduce(struct connection *conn, struct buffered_data *in)
>  		return;
>  	}
>  
> -	if (conn->id != 0 || !conn->can_write) {
> +	if (domain_is_unprivileged(conn) || !conn->can_write) {
>  		send_error(conn, EACCES);
>  		return;
>  	}
> @@ -413,7 +413,7 @@ void do_set_target(struct connection *conn, struct buffered_data *in)
>  		return;
>  	}
>  
> -	if (conn->id != 0 || !conn->can_write) {
> +	if (domain_is_unprivileged(conn) || !conn->can_write) {
>  		send_error(conn, EACCES);
>  		return;
>  	}
> @@ -465,7 +465,7 @@ void do_release(struct connection *conn, const char *domid_str)
>  		return;
>  	}
>  
> -	if (conn->id != 0) {
> +	if (domain_is_unprivileged(conn)) {
>  		send_error(conn, EACCES);
>  		return;
>  	}
> @@ -502,7 +502,7 @@ void do_resume(struct connection *conn, const char *domid_str)
>  		return;
>  	}
>  
> -	if (conn->id != 0) {
> +	if (domain_is_unprivileged(conn)) {
>  		send_error(conn, EACCES);
>  		return;
>  	}