All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Edwin Török" <edvin.torok@citrix.com>,
	"Christian Lindig" <christian.lindig@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>, "Wei Liu" <wl@xen.org>
Subject: [PATCH 2/3] tools/oxenstored: Reject invalid watch paths early
Date: Wed, 3 Feb 2021 17:35:48 +0000	[thread overview]
Message-ID: <20210203173549.21159-3-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20210203173549.21159-1-andrew.cooper3@citrix.com>

From: Edwin Török <edvin.torok@citrix.com>

Watches on invalid paths were accepted, but they would never trigger.  The
client also got no notification that its watch is bad and would never trigger.

Found again by the structured fuzzer, due to an error on live update reload:
the invalid watch paths would get rejected during live update and the list of
watches would be different pre/post live update.

The testcase is watch on `//`, which is an invalid path.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
CC: Christian Lindig <christian.lindig@citrix.com>
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
---
 tools/ocaml/xenstored/connection.ml  | 5 ++---
 tools/ocaml/xenstored/connections.ml | 4 +++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml
index d09a0fa405..65f99ea6f2 100644
--- a/tools/ocaml/xenstored/connection.ml
+++ b/tools/ocaml/xenstored/connection.ml
@@ -158,18 +158,17 @@ let get_children_watches con path =
 let is_dom0 con =
 	Perms.Connection.is_dom0 (get_perm con)
 
-let add_watch con path token =
+let add_watch con (path, apath) token =
 	if !Quota.activate && !Define.maxwatch > 0 &&
 	   not (is_dom0 con) && con.nb_watches > !Define.maxwatch then
 		raise Quota.Limit_reached;
-	let apath = get_watch_path con path in
 	let l = get_watches con apath in
 	if List.exists (fun w -> w.token = token) l then
 		raise Define.Already_exist;
 	let watch = watch_create ~con ~token ~path in
 	Hashtbl.replace con.watches apath (watch :: l);
 	con.nb_watches <- con.nb_watches + 1;
-	apath, watch
+	watch
 
 let del_watch con path token =
 	let apath = get_watch_path con path in
diff --git a/tools/ocaml/xenstored/connections.ml b/tools/ocaml/xenstored/connections.ml
index 8a66eeec3a..3c7429fe7f 100644
--- a/tools/ocaml/xenstored/connections.ml
+++ b/tools/ocaml/xenstored/connections.ml
@@ -114,8 +114,10 @@ let key_of_path path =
 	"" :: Store.Path.to_string_list path
 
 let add_watch cons con path token =
-	let apath, watch = Connection.add_watch con path token in
+	let apath = Connection.get_watch_path con path in
+	(* fail on invalid paths early by calling key_of_str before adding watch *)
 	let key = key_of_str apath in
+	let watch = Connection.add_watch con (path, apath) token in
 	let watches =
  		if Trie.mem cons.watches key
  		then Trie.find cons.watches key
-- 
2.11.0



  parent reply	other threads:[~2021-02-03 17:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-03 17:35 [PATCH for-4.15 0/3] tools/oxenstored bugfixes Andrew Cooper
2021-02-03 17:35 ` [PATCH 1/3] tools/oxenstored: Fix quota calculation for mkdir EEXIST Andrew Cooper
2021-02-03 17:40   ` Ian Jackson
2021-02-03 17:35 ` Andrew Cooper [this message]
2021-02-04 10:51   ` [PATCH 2/3] tools/oxenstored: Reject invalid watch paths early Christian Lindig
2021-02-03 17:35 ` [PATCH 3/3] tools/oxenstored: mkdir conflicts were sometimes missed Andrew Cooper
2021-02-04 10:52   ` Christian Lindig
2021-02-03 17:41 ` [PATCH for-4.15 0/3] tools/oxenstored bugfixes Ian Jackson
2021-02-04 10:46 ` Christian Lindig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210203173549.21159-3-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=christian.lindig@citrix.com \
    --cc=edvin.torok@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.