From nobody Sat May 18 01:59:35 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=quarantine dis=none) header.from=suse.com ARC-Seal: i=1; a=rsa-sha256; t=1645097447; cv=none; d=zohomail.com; s=zohoarc; b=nOb6usCv375i4iNuKlIb1vSRxG8L3ItHl/97kmcwDhhQN+X4LIjLAZsbusQ46C6kU228dg6V+gJJNqdZP0kN1DYzlSJhWXrpP9w2pBFugabAL/TIvPyf2de9YUFDu41BqF6NLVRfhpYqhIPTFIf3TO8+HrtToL9be9HlEjsILNg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1645097447; h=Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=BERprfVQLlUU2/0r8f0rOy+5qxqLqk3fnm/xGVBRun4=; b=esYH8F2BmAh/KZMkAnn4lX5lc0ZtiMMIb1EJD2JNq7jddUX5aJBYR54AfGxT1qwhoUnwR1mbl5hTZ4QWLB/Aed/EDzEmp1zBVC/yeNmDVoNMSoM99Vbys996SwqFO6gcD8jO9hTkaKI7xgjCMOZmBK3ssJsiZ1vChKd0FI2FPPA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=quarantine dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1645097447605331.45053931841994; Thu, 17 Feb 2022 03:30:47 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.274687.470246 (Exim 4.92) (envelope-from ) id 1nKezN-000360-FR; Thu, 17 Feb 2022 11:30:21 +0000 Received: by outflank-mailman (output) from mailman id 274687.470246; Thu, 17 Feb 2022 11:30:21 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nKezN-00035t-Ar; Thu, 17 Feb 2022 11:30:21 +0000 Received: by outflank-mailman (input) for mailman id 274687; Thu, 17 Feb 2022 11:30:20 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nKezM-00035n-PY for xen-devel@lists.xenproject.org; Thu, 17 Feb 2022 11:30:20 +0000 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id fc1f8a24-8fe4-11ec-b215-9bbe72dcb22c; Thu, 17 Feb 2022 12:30:19 +0100 (CET) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 75DE41F37D; Thu, 17 Feb 2022 11:30:18 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 25A4213BBF; Thu, 17 Feb 2022 11:30:18 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id emn0B8oxDmLCXQAAMHmgww (envelope-from ); Thu, 17 Feb 2022 11:30:18 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: fc1f8a24-8fe4-11ec-b215-9bbe72dcb22c DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1645097418; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=BERprfVQLlUU2/0r8f0rOy+5qxqLqk3fnm/xGVBRun4=; b=O1Y0TMtwIkx/68laJyAF3k1ebX172iok7RuvJtJM+mfoUz5ss9R6T2d9x2aox1Fgb7+NVO cHe2A+23FHigp29E5hOKQhBH4n6yEWclO97GWHsFgMcNWxuSYXN3G9KC6Svn9Om9MKESBv djA4EXKFLpt0mLVafUoB0FNpdi6UbGU= From: Juergen Gross To: xen-devel@lists.xenproject.org Cc: Juergen Gross , Andrew Cooper , George Dunlap , Jan Beulich , Julien Grall , Stefano Stabellini , Wei Liu , Anthony PERARD Subject: [PATCH v2] tools/xenstore: add error indicator to ring page Date: Thu, 17 Feb 2022 12:30:16 +0100 Message-Id: <20220217113016.8260-1-jgross@suse.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @suse.com) X-ZM-MESSAGEID: 1645097450485100001 Content-Type: text/plain; charset="utf-8" In case Xenstore is detecting a malicious ring page modification (e.g. an invalid producer or consumer index set by a guest) it will ignore the connection of that guest in future. Add a new error field to the ring page indicating that case. Add a new feature bit in order to signal the presence of that error field. Move the ignore_connection() function to xenstored_domain.c in order to be able to access the ring page for setting the error indicator. Signed-off-by: Juergen Gross Reviewed-by: Anthony PERARD --- V2: - add some clarifications (Anthony PERARD) --- docs/misc/xenstore-ring.txt | 35 +++++++++++++++++++++++++ tools/xenstore/xenstored_core.c | 43 +++++++------------------------ tools/xenstore/xenstored_core.h | 1 - tools/xenstore/xenstored_domain.c | 34 +++++++++++++++++++++++- tools/xenstore/xenstored_domain.h | 1 + xen/include/public/io/xs_wire.h | 9 +++++++ 6 files changed, 88 insertions(+), 35 deletions(-) diff --git a/docs/misc/xenstore-ring.txt b/docs/misc/xenstore-ring.txt index 16b4d0f5ac..b338b21b19 100644 --- a/docs/misc/xenstore-ring.txt +++ b/docs/misc/xenstore-ring.txt @@ -22,6 +22,7 @@ Offset Length Description 2060 4 Output producer offset 2064 4 Server feature bitmap 2068 4 Connection state +2072 4 Connection error indicator =20 The Input data and Output data are circular buffers. Each buffer is associated with a pair of free-running offsets labelled "consumer" and @@ -66,6 +67,7 @@ The following features are defined: Mask Description ----------------------------------------------------------------- 1 Ring reconnection (see the ring reconnection feature below) +2 Connection error indicator (see connection error feature below) =20 The "Connection state" field is used to request a ring close and reconnect. The "Connection state" field only contains valid data if the server has @@ -78,6 +80,19 @@ Value Description 1 Ring close and reconnect is in progress (see the "ring reconnection feature" described below) =20 +The "Connection error indicator" is used to let the server indicate it has +detected some error that led to deactivation of the connection by the serv= er. +If the feature has been advertised then the "Connection error indicator" m= ay +take the following values (new values might be added in future without them +being advertised as a new feature): + +Value Description +----------------------------------------------------------------- +0 No error, connection is valid +1 Communication problems (event channel not functional) +2 Inconsistent producer or consumer offset +3 Protocol violation (client data package too long) + The ring reconnection feature =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D =20 @@ -114,3 +129,23 @@ packet boundary. =20 Note that only the guest may set the Connection state to 1 and only the server may set it back to 0. + +The connection error feature +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D + +The connection error feature allows the server to signal error conditions +leading to a stop of the communication with the client. In case such an er= ror +condition has occurred, the server will set the appropriate error conditio= n in +the Connection error indicator and will stop communication with the client. + +Any value different from 0 is indicating an error. The value used is meant +just for diagnostic purposes. A client reading the error value should be +prepared to see values not described here, as new error cases might be add= ed +in future. + +The server will discard any already read or written packets, in-flight +requests, watches and transactions associated with the connection. + +Depending on the error cause it might be possible that a reconnect via the +ring reconnection feature (if present) can be performed. There is no guara= ntee +this will succeed. diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_cor= e.c index 91d3adccb1..6e4022e5da 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1455,35 +1455,6 @@ static struct { [XS_DIRECTORY_PART] =3D { "DIRECTORY_PART", send_directory_part }, }; =20 -/* - * Keep the connection alive but stop processing any new request or sending - * reponse. This is to allow sending @releaseDomain watch event at the cor= rect - * moment and/or to allow the connection to restart (not yet implemented). - * - * All watches, transactions, buffers will be freed. - */ -void ignore_connection(struct connection *conn) -{ - struct buffered_data *out, *tmp; - - trace("CONN %p ignored\n", conn); - - conn->is_ignored =3D true; - conn_delete_all_watches(conn); - conn_delete_all_transactions(conn); - - list_for_each_entry_safe(out, tmp, &conn->out_list, list) { - list_del(&out->list); - talloc_free(out); - } - - talloc_free(conn->in); - conn->in =3D NULL; - /* if this is a socket connection, drop it now */ - if (conn->fd >=3D 0) - talloc_free(conn); -} - static const char *sockmsg_string(enum xsd_sockmsg_type type) { if ((unsigned int)type < ARRAY_SIZE(wire_funcs) && wire_funcs[type].str) @@ -1598,6 +1569,7 @@ static void handle_input(struct connection *conn) { int bytes; struct buffered_data *in; + unsigned int err; =20 if (!conn->in) { conn->in =3D new_buffer(conn); @@ -1612,8 +1584,10 @@ static void handle_input(struct connection *conn) if (in->used !=3D sizeof(in->hdr)) { bytes =3D conn->funcs->read(conn, in->hdr.raw + in->used, sizeof(in->hdr) - in->used); - if (bytes < 0) + if (bytes < 0) { + err =3D XENSTORE_ERROR_RINGIDX; goto bad_client; + } in->used +=3D bytes; if (in->used !=3D sizeof(in->hdr)) return; @@ -1621,6 +1595,7 @@ static void handle_input(struct connection *conn) if (in->hdr.msg.len > XENSTORE_PAYLOAD_MAX) { syslog(LOG_ERR, "Client tried to feed us %i", in->hdr.msg.len); + err =3D XENSTORE_ERROR_PROTO; goto bad_client; } } @@ -1638,8 +1613,10 @@ static void handle_input(struct connection *conn) =20 bytes =3D conn->funcs->read(conn, in->buffer + in->used, in->hdr.msg.len - in->used); - if (bytes < 0) + if (bytes < 0) { + err =3D XENSTORE_ERROR_RINGIDX; goto bad_client; + } =20 in->used +=3D bytes; if (in->used !=3D in->hdr.msg.len) @@ -1649,14 +1626,14 @@ static void handle_input(struct connection *conn) return; =20 bad_client: - ignore_connection(conn); + ignore_connection(conn, err); } =20 static void handle_output(struct connection *conn) { /* Ignore the connection if an error occured */ if (!write_messages(conn)) - ignore_connection(conn); + ignore_connection(conn, XENSTORE_ERROR_RINGIDX); } =20 struct connection *new_connection(const struct interface_funcs *funcs) diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_cor= e.h index 190d2447cd..742812a974 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -206,7 +206,6 @@ struct node *read_node(struct connection *conn, const v= oid *ctx, =20 struct connection *new_connection(const struct interface_funcs *funcs); struct connection *get_connection_by_id(unsigned int conn_id); -void ignore_connection(struct connection *conn); void check_store(void); void corrupt(struct connection *conn, const char *fmt, ...); =20 diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_d= omain.c index d03c7d93a9..ae065fcbee 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -427,6 +427,38 @@ static void domain_conn_reset(struct domain *domain) domain->interface->rsp_cons =3D domain->interface->rsp_prod =3D 0; } =20 +/* + * Keep the connection alive but stop processing any new request or sending + * reponse. This is to allow sending @releaseDomain watch event at the cor= rect + * moment and/or to allow the connection to restart (not yet implemented). + * + * All watches, transactions, buffers will be freed. + */ +void ignore_connection(struct connection *conn, unsigned int err) +{ + struct buffered_data *out, *tmp; + + trace("CONN %p ignored, reason %u\n", conn, err); + + if (conn->domain && conn->domain->interface) + conn->domain->interface->error =3D err; + + conn->is_ignored =3D true; + conn_delete_all_watches(conn); + conn_delete_all_transactions(conn); + + list_for_each_entry_safe(out, tmp, &conn->out_list, list) { + list_del(&out->list); + talloc_free(out); + } + + talloc_free(conn->in); + conn->in =3D NULL; + /* if this is a socket connection, drop it now */ + if (conn->fd >=3D 0) + talloc_free(conn); +} + static struct domain *introduce_domain(const void *ctx, unsigned int domid, evtchn_port_t port, bool restore) @@ -1305,7 +1337,7 @@ void read_state_connection(const void *ctx, const voi= d *state) * dead. So mark it as ignored. */ if (!domain->port || !domain->interface) - ignore_connection(conn); + ignore_connection(conn, XENSTORE_ERROR_COMM); =20 if (sc->spec.ring.tdomid !=3D DOMID_INVALID) { tdomain =3D find_or_alloc_domain(ctx, diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_d= omain.h index 1e929b8f8c..4a37de67a0 100644 --- a/tools/xenstore/xenstored_domain.h +++ b/tools/xenstore/xenstored_domain.h @@ -47,6 +47,7 @@ int do_reset_watches(struct connection *conn, struct buff= ered_data *in); void domain_init(int evtfd); void dom0_init(void); void domain_deinit(void); +void ignore_connection(struct connection *conn, unsigned int err); =20 /* Returns the implicit path of a connection (only domains have this) */ const char *get_implicit_path(const struct connection *conn); diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wir= e.h index 4dd6632669..953a0050a3 100644 --- a/xen/include/public/io/xs_wire.h +++ b/xen/include/public/io/xs_wire.h @@ -124,6 +124,7 @@ struct xenstore_domain_interface { XENSTORE_RING_IDX rsp_cons, rsp_prod; uint32_t server_features; /* Bitmap of features supported by the serve= r */ uint32_t connection; + uint32_t error; }; =20 /* Violating this is very bad. See docs/misc/xenstore.txt. */ @@ -135,11 +136,19 @@ struct xenstore_domain_interface { =20 /* The ability to reconnect a ring */ #define XENSTORE_SERVER_FEATURE_RECONNECTION 1 +/* The presence of the "error" field in the ring page */ +#define XENSTORE_SERVER_FEATURE_ERROR 2 =20 /* Valid values for the connection field */ #define XENSTORE_CONNECTED 0 /* the steady-state */ #define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */ =20 +/* Valid values for the error field */ +#define XENSTORE_ERROR_NONE 0 /* No error */ +#define XENSTORE_ERROR_COMM 1 /* Communication problem */ +#define XENSTORE_ERROR_RINGIDX 2 /* Invalid ring index */ +#define XENSTORE_ERROR_PROTO 3 /* Protocol violation (payload too long) = */ + #endif /* _XS_WIRE_H */ =20 /* --=20 2.34.1