1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
|
From: Chris Liddell <chris.liddell@artifex.com>
Date: Fri, 24 Aug 2018 08:26:04 +0000 (+0100)
Subject: Improve restore robustness
X-Git-Tag: ghostpdl-9.24rc1~10
X-Git-Url: http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff_plain;h=5516c614
Improve restore robustness
Prompted by looking at Bug 699654:
There are two variants of the restore operator in Ghostscript: one is Level 1
(restoring VM), the other is Level 2+ (adding page device restoring to the
Level operator).
This was implemented by the Level 2+ version restoring the device in the
graphics state, then calling the Level 1 implementation to handle actually
restoring the VM state.
The problem was that the operand checking, and sanity of the save object was
only done by the Level 1 variant, thus meaning an invalid save object could
leave a (Level 2+) restore partially complete - with the page device part
restored, but not VM, and the page device not configured.
To solve that, this commit splits the operand and sanity checking, and the
core of the restore operation into separate functions, so the relevant
operators can validate the operand *before* taking any further action. That
reduces the chances of an invalid restore leaving the interpreter in an
unknown state.
If an error occurs during the actual VM restore it is essentially fatal, and the
interpreter cannot continue, but as an extra surety for security, in the event
of such an error, we'll explicitly preserve the LockSafetyParams of the device,
rather than rely on the post-restore device configuration (which won't happen
in the event of an error).
---
diff --git a/psi/int.mak b/psi/int.mak
index 1968820..16db0cf 100644
--- a/psi/int.mak
+++ b/psi/int.mak
@@ -1086,8 +1086,8 @@ $(PSD)pagedev.dev : $(ECHOGS_XE) $(pagedev_)\
$(PSOBJ)zdevice2.$(OBJ) : $(PSSRC)zdevice2.c $(OP) $(math__h) $(memory__h)\
$(dstack_h) $(estack_h)\
- $(idict_h) $(idparam_h) $(igstate_h) $(iname_h) $(iutil_h) $(store_h)\
- $(gxdevice_h) $(gsstate_h) $(INT_MAK) $(MAKEDIRS)
+ $(idict_h) $(idparam_h) $(igstate_h) $(iname_h) $(isave) $(iutil_h) \
+ $(store_h) $(gxdevice_h) $(gsstate_h) $(INT_MAK) $(MAKEDIRS)
$(PSCC) $(PSO_)zdevice2.$(OBJ) $(C_) $(PSSRC)zdevice2.c
$(PSOBJ)zmedia2.$(OBJ) : $(PSSRC)zmedia2.c $(OP) $(math__h) $(memory__h)\
diff --git a/psi/isave.h b/psi/isave.h
index 3021639..7eaaced 100644
--- a/psi/isave.h
+++ b/psi/isave.h
@@ -128,4 +128,10 @@ int font_restore(const alloc_save_t * save);
express purpose of getting the library context. */
gs_memory_t *gs_save_any_memory(const alloc_save_t *save);
+int
+restore_check_save(i_ctx_t *i_ctx_p, alloc_save_t **asave);
+
+int
+dorestore(i_ctx_t *i_ctx_p, alloc_save_t *asave);
+
#endif /* isave_INCLUDED */
diff --git a/psi/zdevice2.c b/psi/zdevice2.c
index 9fbb4e3..0c7080d 100644
--- a/psi/zdevice2.c
+++ b/psi/zdevice2.c
@@ -26,6 +26,7 @@
#include "igstate.h"
#include "iname.h"
#include "iutil.h"
+#include "isave.h"
#include "store.h"
#include "gxdevice.h"
#include "gsstate.h"
@@ -307,13 +308,24 @@ z2grestoreall(i_ctx_t *i_ctx_p)
}
return 0;
}
-
+/* This is the Level 2+ variant of restore - which adds restoring
+ of the page device to the Level 1 variant in zvmem.c.
+ Previous this restored the device state before calling zrestore.c
+ which validated operands etc, meaning a restore could error out
+ partially complete.
+ The operand checking, and actual VM restore are now in two functions
+ so they can called separately thus, here, we can do as much
+ checking as possible, before embarking on actual changes
+ */
/* <save> restore - */
static int
z2restore(i_ctx_t *i_ctx_p)
{
- os_ptr op = osp;
- check_type(*op, t_save);
+ alloc_save_t *asave;
+ bool saveLockSafety = gs_currentdevice_inline(igs)->LockSafetyParams;
+ int code = restore_check_save(i_ctx_p, &asave);
+
+ if (code < 0) return code;
while (gs_gstate_saved(gs_gstate_saved(igs))) {
if (restore_page_device(igs, gs_gstate_saved(igs)))
@@ -322,7 +334,20 @@ z2restore(i_ctx_t *i_ctx_p)
}
if (restore_page_device(igs, gs_gstate_saved(igs)))
return push_callout(i_ctx_p, "%restorepagedevice");
- return zrestore(i_ctx_p);
+
+ code = dorestore(i_ctx_p, asave);
+
+ if (code < 0) {
+ /* An error here is basically fatal, but....
+ restore_page_device() has to set LockSafetyParams false so it can
+ configure the restored device correctly - in normal operation, that
+ gets reset by that configuration. If we hit an error, though, that
+ may not happen - at least ensure we keep the setting through the
+ error.
+ */
+ gs_currentdevice_inline(igs)->LockSafetyParams = saveLockSafety;
+ }
+ return code;
}
/* <gstate> setgstate - */
diff --git a/psi/zvmem.c b/psi/zvmem.c
index 44cd7a8..87a0a4f 100644
--- a/psi/zvmem.c
+++ b/psi/zvmem.c
@@ -99,19 +99,18 @@ zsave(i_ctx_t *i_ctx_p)
static int restore_check_operand(os_ptr, alloc_save_t **, gs_dual_memory_t *);
static int restore_check_stack(const i_ctx_t *i_ctx_p, const ref_stack_t *, const alloc_save_t *, bool);
static void restore_fix_stack(i_ctx_t *i_ctx_p, ref_stack_t *, const alloc_save_t *, bool);
+
+/* Do as many up front checks of the save object as we reasonably can */
int
-zrestore(i_ctx_t *i_ctx_p)
+restore_check_save(i_ctx_t *i_ctx_p, alloc_save_t **asave)
{
os_ptr op = osp;
- alloc_save_t *asave;
- bool last;
- vm_save_t *vmsave;
- int code = restore_check_operand(op, &asave, idmemory);
+ int code = restore_check_operand(op, asave, idmemory);
if (code < 0)
return code;
if_debug2m('u', imemory, "[u]vmrestore 0x%lx, id = %lu\n",
- (ulong) alloc_save_client_data(asave),
+ (ulong) alloc_save_client_data(*asave),
(ulong) op->value.saveid);
if (I_VALIDATE_BEFORE_RESTORE)
ivalidate_clean_spaces(i_ctx_p);
@@ -120,14 +119,37 @@ zrestore(i_ctx_t *i_ctx_p)
{
int code;
- if ((code = restore_check_stack(i_ctx_p, &o_stack, asave, false)) < 0 ||
- (code = restore_check_stack(i_ctx_p, &e_stack, asave, true)) < 0 ||
- (code = restore_check_stack(i_ctx_p, &d_stack, asave, false)) < 0
+ if ((code = restore_check_stack(i_ctx_p, &o_stack, *asave, false)) < 0 ||
+ (code = restore_check_stack(i_ctx_p, &e_stack, *asave, true)) < 0 ||
+ (code = restore_check_stack(i_ctx_p, &d_stack, *asave, false)) < 0
) {
osp++;
return code;
}
}
+ osp++;
+ return 0;
+}
+
+/* the semantics of restore differ slightly between Level 1 and
+ Level 2 and later - the latter includes restoring the device
+ state (whilst Level 1 didn't have "page devices" as such).
+ Hence we have two restore operators - one here (Level 1)
+ and one in zdevice2.c (Level 2+). For that reason, the
+ operand checking and guts of the restore operation are
+ separated so both implementations can use them to best
+ effect.
+ */
+int
+dorestore(i_ctx_t *i_ctx_p, alloc_save_t *asave)
+{
+ os_ptr op = osp;
+ bool last;
+ vm_save_t *vmsave;
+ int code;
+
+ osp--;
+
/* Reset l_new in all stack entries if the new save level is zero. */
/* Also do some special fixing on the e-stack. */
restore_fix_stack(i_ctx_p, &o_stack, asave, false);
@@ -170,9 +192,24 @@ zrestore(i_ctx_t *i_ctx_p)
/* cause an 'invalidaccess' in setuserparams. Temporarily set */
/* LockFilePermissions false until the gs_lev2.ps can do a */
/* setuserparams from the restored userparam dictionary. */
+ /* NOTE: This is safe to do here, since the restore has */
+ /* successfully completed - this should never come before any */
+ /* operation that can trigger an error */
i_ctx_p->LockFilePermissions = false;
return 0;
}
+
+int
+zrestore(i_ctx_t *i_ctx_p)
+{
+ alloc_save_t *asave;
+ int code = restore_check_save(i_ctx_p, &asave);
+ if (code < 0)
+ return code;
+
+ return dorestore(i_ctx_p, asave);
+}
+
/* Check the operand of a restore. */
static int
restore_check_operand(os_ptr op, alloc_save_t ** pasave,
@@ -193,6 +230,7 @@ restore_check_operand(os_ptr op, alloc_save_t ** pasave,
*pasave = asave;
return 0;
}
+
/* Check a stack to make sure all its elements are older than a save. */
static int
restore_check_stack(const i_ctx_t *i_ctx_p, const ref_stack_t * pstack,
|
