Skip to content

Commit f3beb44

Browse files
author
Christian Lindig
committed
CA-382035: Verify commandline items for service process in "pid"
Backport of 9ea53a0d033d1f76b1d2f35e59d7b61669db7d46 in xen-api.git. The code on the master branch is organised differently. Hence, this is a manual backport of the implementation. When xenopsd-xc getting process id of a service, verify that its commandline items include expected process name and some commandline arg containing domid if domid is not part of its process name. Signed-off-by: Gang Ji <gang.ji@citrix.com> Signed-off-by: Christian Lindig <christian.lindig@cloud.com>
1 parent 089ca98 commit f3beb44

File tree

1 file changed

+82
-24
lines changed

1 file changed

+82
-24
lines changed

xc/device.ml

Lines changed: 82 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,16 @@ module type DAEMONPIDPATH = sig
10771077
val use_pidfile : bool
10781078

10791079
val pid_path : int -> string
1080+
1081+
(* To check if a pid is valid, look up its process name, and some commandline
1082+
arg containing domid if domid is not part of its process name, check that
1083+
they are contained in /proc/<pid>/cmdline.
1084+
The expected cmdline items are set by (expected_cmdline_items domid) for
1085+
each service. Note that the parameter "domid" here is required as
1086+
otherwise we can not distinguish between the process instances of the same
1087+
service for different domains. *)
1088+
val expected_cmdline_items : domid:int -> string list
1089+
10801090
end
10811091

10821092
module DaemonMgmt (D : DAEMONPIDPATH) = struct
@@ -1108,29 +1118,59 @@ module DaemonMgmt (D : DAEMONPIDPATH) = struct
11081118
else
11091119
None
11101120

1111-
let pid ~xs domid =
1121+
(* For process id, look up its process name, and some commandline arg
1122+
containing domid if domid is not part of its process name, check that
1123+
they are contained in /proc/<pid>/cmdline. *)
1124+
let is_cmdline_valid ~pid expected_args =
11121125
try
1113-
match pidfile_path domid with
1114-
| Some path when Sys.file_exists path ->
1115-
let pid =
1116-
path |> Unixext.string_of_file |> String.trim |> int_of_string
1117-
in
1118-
Unixext.with_file path [Unix.O_RDONLY] 0 (fun fd ->
1119-
try
1120-
Unix.lockf fd Unix.F_TRLOCK 0 ;
1121-
(* we succeeded taking the lock: original process is dead.
1122-
* some other process might've reused its pid *)
1123-
None
1124-
with Unix.Unix_error (Unix.EAGAIN, _, _) ->
1125-
(* cannot obtain lock: process is alive *)
1126-
Some pid
1127-
)
1126+
let cmdline_str =
1127+
Printf.sprintf "/proc/%d/cmdline" pid |> Unixext.string_of_file
1128+
in
1129+
match cmdline_str with
1130+
| "" ->
1131+
(* from man proc:
1132+
/proc/[pid]/cmdline
1133+
This read-only file holds the complete command line for the process,
1134+
unless the process is a zombie. In the latter case, there is nothing
1135+
in this file: that is, a read on this file will return 0 characters.
1136+
This applies when the VM is being shut down. *)
1137+
false
11281138
| _ ->
1129-
(* backward compatibility during update installation: only has
1130-
xenstore pid *)
1131-
let pid = xs.Xs.read (pid_path domid) in
1132-
Some (int_of_string pid)
1133-
with _ -> None
1139+
let cmdline = Astring.String.cuts ~sep:"\000" cmdline_str in
1140+
let valid =
1141+
List.for_all (fun arg -> List.mem arg cmdline) expected_args
1142+
in
1143+
if not valid then error "%s: pid not valid (pid = %d)" D.name pid;
1144+
valid
1145+
with _ -> false
1146+
1147+
let pid ~xs domid =
1148+
let ( let* ) = Option.bind in
1149+
let* pid =
1150+
try
1151+
match pidfile_path domid with
1152+
| Some path when Sys.file_exists path ->
1153+
let pid =
1154+
path |> Unixext.string_of_file |> String.trim |> int_of_string
1155+
in
1156+
Unixext.with_file path [ Unix.O_RDONLY ] 0 (fun fd ->
1157+
try
1158+
Unix.lockf fd Unix.F_TRLOCK 0;
1159+
(* we succeeded taking the lock: original process is dead.
1160+
* some other process might've reused its pid *)
1161+
None
1162+
with Unix.Unix_error (Unix.EAGAIN, _, _) ->
1163+
(* cannot obtain lock: process is alive *)
1164+
Some pid)
1165+
| _ ->
1166+
(* backward compatibility during update installation: only has
1167+
xenstore pid *)
1168+
let pid = xs.Xs.read (pid_path domid) in
1169+
Some (int_of_string pid)
1170+
with _ -> None
1171+
in
1172+
if is_cmdline_valid ~pid (D.expected_cmdline_items ~domid) then Some pid
1173+
else None
11341174

11351175
let is_running ~xs domid =
11361176
match pid ~xs domid with
@@ -1254,6 +1294,9 @@ module Qemu = DaemonMgmt (struct
12541294
let use_pidfile = true
12551295

12561296
let pid_path domid = sprintf "/local/domain/%d/qemu-pid" domid
1297+
1298+
let expected_cmdline_items ~domid = [Printf.sprintf "qemu-dm-%d" domid]
1299+
12571300
end)
12581301

12591302
module Vgpu = DaemonMgmt (struct
@@ -1262,6 +1305,10 @@ module Vgpu = DaemonMgmt (struct
12621305
let use_pidfile = false
12631306

12641307
let pid_path domid = sprintf "/local/domain/%d/vgpu-pid" domid
1308+
1309+
let domain_arg domid = Printf.sprintf "--domain=%d" domid
1310+
1311+
let expected_cmdline_items ~domid = [!Xc_resources.vgpu; domain_arg domid]
12651312
end)
12661313

12671314
module Varstored = SystemdDaemonMgmt (struct
@@ -1270,18 +1317,29 @@ module Varstored = SystemdDaemonMgmt (struct
12701317
let use_pidfile = true
12711318

12721319
let pid_path domid = sprintf "/local/domain/%d/varstored-pid" domid
1320+
1321+
let expected_cmdline_items ~domid =
1322+
[!Xc_resources.varstored; pid_path domid]
12731323
end)
12741324

12751325
module PV_Vnc = struct
1326+
1327+
let vnc_console_path domid = sprintf "/local/domain/%d/console" domid
1328+
12761329
module D = DaemonMgmt (struct
12771330
let name = "vncterm"
12781331

12791332
let use_pidfile = false
12801333

12811334
let pid_path domid = sprintf "/local/domain/%d/vncterm-pid" domid
1335+
1336+
let expected_cmdline_items ~domid =
1337+
[
1338+
!Xc_resources.vncterm (* vncterm binary path *)
1339+
; vnc_console_path domid (* xenstore console path *)
1340+
]
12821341
end)
12831342

1284-
let vnc_console_path domid = sprintf "/local/domain/%d/console" domid
12851343

12861344
let pid ~xs domid = D.pid ~xs domid
12871345

@@ -1307,7 +1365,7 @@ module PV_Vnc = struct
13071365
D.is_running ~xs domid && is_cmdline_valid domid p
13081366

13091367
let get_vnc_port ~xs domid =
1310-
if not (is_vncterm_running ~xs domid) then
1368+
if not (D.is_running ~xs domid) then
13111369
None
13121370
else
13131371
try
@@ -1317,7 +1375,7 @@ module PV_Vnc = struct
13171375
with _ -> None
13181376

13191377
let get_tc_port ~xs domid =
1320-
if not (is_vncterm_running ~xs domid) then
1378+
if not (D.is_running ~xs domid) then
13211379
None
13221380
else
13231381
try Some (int_of_string (xs.Xs.read (Generic.tc_port_path domid)))

0 commit comments

Comments
 (0)