diff mbox series

[04/10] tools/xenstored: Limit the number of requests a connection can delay

Message ID 20210616144324.31652-5-julien@xen.org (mailing list archive)
State New
Headers show
Series tools/xenstored: Bug fixes + Improve Live-Update | expand

Commit Message

Julien Grall June 16, 2021, 2:43 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

Currently, only liveupdate request can be delayed. The request can only
be performed by a privileged connection (e.g. dom0). So it is fine to
have no limits.

In a follow-up patch we will want to delay request for unprivileged
connection as well. So it is best to apply a limit.

For now and for simplicity, only a single request can be delayed
for a given unprivileged connection.

Take the opportunity to tweak the prototype and provide a way to
bypass the quota check. This would be useful when the function
is called from the restore code.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 tools/xenstore/xenstored_control.c |  2 +-
 tools/xenstore/xenstored_core.c    | 11 ++++++++++-
 tools/xenstore/xenstored_core.h    |  3 ++-
 3 files changed, 13 insertions(+), 3 deletions(-)

Comments

Luca Fancellu June 21, 2021, 9:02 a.m. UTC | #1
> On 16 Jun 2021, at 15:43, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, only liveupdate request can be delayed. The request can only
> be performed by a privileged connection (e.g. dom0). So it is fine to
> have no limits.
> 
> In a follow-up patch we will want to delay request for unprivileged
> connection as well. So it is best to apply a limit.
> 
> For now and for simplicity, only a single request can be delayed
> for a given unprivileged connection.
> 
> Take the opportunity to tweak the prototype and provide a way to
> bypass the quota check. This would be useful when the function
> is called from the restore code.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> ---
> tools/xenstore/xenstored_control.c |  2 +-
> tools/xenstore/xenstored_core.c    | 11 ++++++++++-
> tools/xenstore/xenstored_core.h    |  3 ++-
> 3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
> index 7acc2d134f9f..1c24d4869eab 100644
> --- a/tools/xenstore/xenstored_control.c
> +++ b/tools/xenstore/xenstored_control.c
> @@ -737,7 +737,7 @@ static const char *lu_start(const void *ctx, struct connection *conn,
> 	lu_status->timeout = to;
> 	lu_status->started_at = time(NULL);
> 
> -	errno = delay_request(conn, conn->in, do_lu_start, NULL);
> +	errno = delay_request(conn, conn->in, do_lu_start, NULL, false);
> 
> 	return NULL;
> }
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 41b26d7094c8..51d210828922 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -279,10 +279,19 @@ static void call_delayed(struct connection *conn, struct delayed_request *req)
> }
> 
> int delay_request(struct connection *conn, struct buffered_data *in,
> -		  bool (*func)(struct delayed_request *), void *data)
> +		  bool (*func)(struct delayed_request *), void *data,
> +		  bool no_quota_check)
> {
> 	struct delayed_request *req;
> 
> +	/*
> +	 * Only allow one request can be delayed for an unprivileged
> +	 * connection.
> +	 */
> +	if (!no_quota_check && domain_is_unprivileged(conn) &&
> +	    !list_empty(&conn->delayed))
> +		return ENOSPC;
> +
> 	req = talloc(in, struct delayed_request);
> 	if (!req)
> 		return ENOMEM;
> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
> index 89ce155e755b..34839b34f6e9 100644
> --- a/tools/xenstore/xenstored_core.h
> +++ b/tools/xenstore/xenstored_core.h
> @@ -213,7 +213,8 @@ char *get_parent(const void *ctx, const char *node);
> 
> /* Delay a request. */
> int delay_request(struct connection *conn, struct buffered_data *in,
> -		  bool (*func)(struct delayed_request *), void *data);
> +		  bool (*func)(struct delayed_request *), void *data,
> +		  bool no_quota_check);
> 
> /* Tracing infrastructure. */
> void trace_create(const void *data, const char *type);
> -- 
> 2.17.1
> 
>
Juergen Gross June 24, 2021, 7:35 a.m. UTC | #2
On 16.06.21 16:43, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, only liveupdate request can be delayed. The request can only
> be performed by a privileged connection (e.g. dom0). So it is fine to
> have no limits.
> 
> In a follow-up patch we will want to delay request for unprivileged
> connection as well. So it is best to apply a limit.
> 
> For now and for simplicity, only a single request can be delayed
> for a given unprivileged connection.
> 
> Take the opportunity to tweak the prototype and provide a way to
> bypass the quota check. This would be useful when the function
> is called from the restore code.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen
diff mbox series

Patch

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 7acc2d134f9f..1c24d4869eab 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -737,7 +737,7 @@  static const char *lu_start(const void *ctx, struct connection *conn,
 	lu_status->timeout = to;
 	lu_status->started_at = time(NULL);
 
-	errno = delay_request(conn, conn->in, do_lu_start, NULL);
+	errno = delay_request(conn, conn->in, do_lu_start, NULL, false);
 
 	return NULL;
 }
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 41b26d7094c8..51d210828922 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -279,10 +279,19 @@  static void call_delayed(struct connection *conn, struct delayed_request *req)
 }
 
 int delay_request(struct connection *conn, struct buffered_data *in,
-		  bool (*func)(struct delayed_request *), void *data)
+		  bool (*func)(struct delayed_request *), void *data,
+		  bool no_quota_check)
 {
 	struct delayed_request *req;
 
+	/*
+	 * Only allow one request can be delayed for an unprivileged
+	 * connection.
+	 */
+	if (!no_quota_check && domain_is_unprivileged(conn) &&
+	    !list_empty(&conn->delayed))
+		return ENOSPC;
+
 	req = talloc(in, struct delayed_request);
 	if (!req)
 		return ENOMEM;
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 89ce155e755b..34839b34f6e9 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -213,7 +213,8 @@  char *get_parent(const void *ctx, const char *node);
 
 /* Delay a request. */
 int delay_request(struct connection *conn, struct buffered_data *in,
-		  bool (*func)(struct delayed_request *), void *data);
+		  bool (*func)(struct delayed_request *), void *data,
+		  bool no_quota_check);
 
 /* Tracing infrastructure. */
 void trace_create(const void *data, const char *type);