Package: apparmor / 2.13.2-10

upstream-commit-394d086-parser-Fix-parser-failing-to-handle-errors-when-setting-u.patch Patch series | download
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
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 20 Feb 2019 01:17:06 -0800
Subject: parser: Fix parser failing to handle errors when setting up work

The parser is not correctly handling some error conditions when
dealing with work units. Failure to spawn work, access files, etc
should be returned where appropriate, and be able to abort processing
if abort_on_error is set.

In addition some errors are leading to a direct exit without checking
for abort_on_error.

BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=921866
BugLink: http://bugs.launchpad.net/bugs/1815294

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Eric Chiang <ericchiang@google.com>
(backported from commit cb43e57d27962039c5bc2a380936c7316575701f)

Conflicts:
	parser/parser_main.c
Reason:
	commit 48a32b78b189cf9e2c4d8bce8fb45c68bf4cc327 not backported
---
 parser/parser.h      | 14 +++++++++++--
 parser/parser_main.c | 57 ++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/parser/parser.h b/parser/parser.h
index e7acda6..5643f55 100644
--- a/parser/parser.h
+++ b/parser/parser.h
@@ -171,13 +171,23 @@ extern int preprocess_only;
 
 
 #ifdef DEBUG
-#define PDEBUG(fmt, args...) fprintf(stderr, "parser: " fmt, ## args)
+#define PDEBUG(fmt, args...)				\
+do {							\
+	int pdebug_error = errno;			\
+	fprintf(stderr, "parser: " fmt, ## args);	\
+	errno = pdebug_error;				\
+} while (0)
 #else
 #define PDEBUG(fmt, args...)	/* Do nothing */
 #endif
 #define NPDEBUG(fmt, args...)	/* Do nothing */
 
-#define PERROR(fmt, args...) fprintf(stderr, fmt, ## args)
+#define PERROR(fmt, args...)			\
+do {						\
+	int perror_error = errno;		\
+	fprintf(stderr, fmt, ## args);		\
+	errno = perror_error;			\
+} while (0)
 
 #ifndef TRUE
 #define TRUE	(1)
diff --git a/parser/parser_main.c b/parser/parser_main.c
index 6260d02..0e3666b 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -1088,8 +1088,11 @@ do {									\
 		work_sync_one(RESULT);					\
 } while (0)
 
+/* returns -1 if work_spawn fails, not a return value of any unit of work */
 #define work_spawn(WORK, RESULT)					\
-do {									\
+({									\
+	int localrc = 0;						\
+	do {								\
 	/* what to do to avoid fork() overhead when single threaded	\
 	if (jobs == 1) {						\
 		// no parallel work so avoid fork() overhead		\
@@ -1126,11 +1129,17 @@ do {									\
 			fprintf(stderr, "    JOBS SPAWN: created %ld ...\n", njobs);									\
 	} else {							\
 		/* error */						\
-		if (debug_jobs)						\
-			fprintf(stderr, "    JOBS SPAWN: failed error: %d) ...\n", errno);								\
+		if (debug_jobs)	{					\
+			int error = errno;				\
+			fprintf(stderr, "    JOBS SPAWN: failed error: %d) ...\n", errno);	\
+			errno = error;					\
+		}							\
 		RESULT(errno);						\
+		localrc = -1;						\
 	}								\
-} while (0)
+	} while (0);							\
+	localrc;							\
+})
 
 
 /* sadly C forces us to do this with exit, long_jump or returning error
@@ -1207,11 +1216,15 @@ static int profile_dir_cb(int dirfd unused, const char *name, struct stat *st,
 	if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) {
 		struct dir_cb_data *cb_data = (struct dir_cb_data *)data;
 		autofree char *path = NULL;
-		if (asprintf(&path, "%s/%s", cb_data->dirname, name) < 0)
+		if (asprintf(&path, "%s/%s", cb_data->dirname, name) < 0) {
 			PERROR(_("Out of memory"));
-		work_spawn(process_profile(option, cb_data->kernel_interface,
-					   path, cb_data->policy_cache),
-			   handle_work_result);
+			handle_work_result(errno);
+			return -1;
+		}
+		rc = work_spawn(process_profile(option,
+						cb_data->kernel_interface,
+						path, cb_data->policy_cache),
+				handle_work_result);
 	}
 	return rc;
 }
@@ -1225,11 +1238,15 @@ static int binary_dir_cb(int dirfd unused, const char *name, struct stat *st,
 	if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) {
 		struct dir_cb_data *cb_data = (struct dir_cb_data *)data;
 		autofree char *path = NULL;
-		if (asprintf(&path, "%s/%s", cb_data->dirname, name) < 0)
+		if (asprintf(&path, "%s/%s", cb_data->dirname, name) < 0) {
 			PERROR(_("Out of memory"));
-		work_spawn(process_binary(option, cb_data->kernel_interface,
-					   path),
-			    handle_work_result);
+			handle_work_result(errno);
+			return -1;
+		}
+		rc = work_spawn(process_binary(option,
+					       cb_data->kernel_interface,
+					       path),
+				handle_work_result);
 	}
 	return rc;
 }
@@ -1359,11 +1376,14 @@ int main(int argc, char *argv[])
 		}
 		/* skip stdin if we've seen other command line arguments */
 		if (i == argc && optind != argc)
-			continue;
+			goto cleanup;
 
 		if (profilename && stat(profilename, &stat_file) == -1) {
+			last_error = errno;
 			PERROR("File %s not found, skipping...\n", profilename);
-			continue;
+			if (abort_on_error)
+				break;
+			goto cleanup;
 		}
 
 		if (profilename && S_ISDIR(stat_file.st_mode)) {
@@ -1378,20 +1398,27 @@ int main(int argc, char *argv[])
 			cb = binary_input ? binary_dir_cb : profile_dir_cb;
 			if ((retval = dirat_for_each(AT_FDCWD, profilename,
 						     &cb_data, cb))) {
+				last_error = errno;
 				PDEBUG("Failed loading profiles from %s\n",
 				       profilename);
+				if (abort_on_error)
+					break;
 			}
 		} else if (binary_input) {
+			/* ignore return as error is handled in work_spawn */
 			work_spawn(process_binary(option, kernel_interface,
 						  profilename),
 				   handle_work_result);
 		} else {
+			/* ignore return as error is handled in work_spawn */
 			work_spawn(process_profile(option, kernel_interface,
 						   profilename, policy_cache),
 				   handle_work_result);
 		}
 
-		if (profilename) free(profilename);
+	cleanup:
+		if (profilename)
+			free(profilename);
 		profilename = NULL;
 	}
 	work_sync(handle_work_result);