Merge lp:~colin-king/ecryptfs/misc-fixes into lp:ecryptfs

Proposed by Colin Ian King
Status: Merged
Merged at revision: 777
Proposed branch: lp:~colin-king/ecryptfs/misc-fixes
Merge into: lp:ecryptfs
Diff against target: 234 lines (+41/-20)
9 files modified
src/libecryptfs/decision_graph.c (+2/-0)
src/libecryptfs/key_management.c (+0/-1)
src/libecryptfs/main.c (+2/-0)
src/libecryptfs/module_mgr.c (+1/-1)
src/utils/io.c (+2/-1)
src/utils/mount.ecryptfs_private.c (+12/-5)
tests/kernel/inotify/test.c (+16/-8)
tests/kernel/trunc-file/test.c (+5/-3)
tests/userspace/wrap-unwrap/test.c (+1/-1)
To merge this branch: bzr merge lp:~colin-king/ecryptfs/misc-fixes
Reviewer Review Type Date Requested Status
Tyler Hicks Approve
Review via email: mp+168057@code.launchpad.net

Description of the change

I've found a bunch of minor bugs which I've fixed up and I've given them a cursory check to see if they are OK and don't regress eCryptfs but I'd like somebody to eyeball them before merging if that's OK.

To post a comment you must log in.
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Thanks, Colin! The only thing that caught my eye was the off_t to size_t changes. Since we're dealing with a file size, we should be using off_t rather than ssize_t (ssize_t wraps at 2GB on 32 bit machines). But, I see where other portions of that test use ssize_t so this change doesn't make us any worse off than what we already are at this point.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/libecryptfs/decision_graph.c'
2--- src/libecryptfs/decision_graph.c 2009-05-29 14:03:56 +0000
3+++ src/libecryptfs/decision_graph.c 2013-06-07 13:34:31 +0000
4@@ -949,6 +949,7 @@
5 param_node->val))) {
6 syslog(LOG_ERR, "%s: Cannot find key_mod for param_node with "
7 "val = [%s]\n", __FUNCTION__, param_node->val);
8+ free(subgraph_ctx);
9 goto out;
10 }
11 (*foo) = (void *)subgraph_ctx;
12@@ -984,6 +985,7 @@
13 memset(val_node, 0, sizeof(struct val_node));
14 if ((rc = asprintf((char **)&val_node->val, "%s", param_node->val))
15 == -1) {
16+ free(val_node);
17 rc = -ENOMEM;
18 goto out;
19 }
20
21=== modified file 'src/libecryptfs/key_management.c'
22--- src/libecryptfs/key_management.c 2012-10-03 19:58:04 +0000
23+++ src/libecryptfs/key_management.c 2013-06-07 13:34:31 +0000
24@@ -257,7 +257,6 @@
25 syslog(LOG_ERR, "Error attempting to open [%s] for reading\n",
26 src);
27 rc = -EIO;
28- close(fd);
29 goto out;
30 }
31 if ((size = read(fd, decrypted_passphrase,
32
33=== modified file 'src/libecryptfs/main.c'
34--- src/libecryptfs/main.c 2012-04-26 21:03:38 +0000
35+++ src/libecryptfs/main.c 2013-06-07 13:34:31 +0000
36@@ -116,10 +116,12 @@
37 } else {
38 flockfile(fh);
39 if ((mnt = (char *)malloc(MAXPATHLEN+1)) == NULL) {
40+ fclose(fh);
41 perror("malloc");
42 return NULL;
43 }
44 if (fgets(mnt, MAXPATHLEN, fh) == NULL) {
45+ free(mnt);
46 mnt = mnt_default;
47 } else {
48 /* Ensure that mnt doesn't contain newlines */
49
50=== modified file 'src/libecryptfs/module_mgr.c'
51--- src/libecryptfs/module_mgr.c 2012-07-11 09:12:11 +0000
52+++ src/libecryptfs/module_mgr.c 2013-06-07 13:34:31 +0000
53@@ -578,7 +578,7 @@
54 static int init_ecryptfs_cipher_param_node()
55 {
56 struct cipher_descriptor *cd = cipher_descriptors;
57- int rc;
58+ int rc = 0;
59
60 while (cd && cd->name) {
61 struct transition_node *tn;
62
63=== modified file 'src/utils/io.c'
64--- src/utils/io.c 2009-05-29 14:03:56 +0000
65+++ src/utils/io.c 2013-06-07 13:34:31 +0000
66@@ -201,7 +201,7 @@
67 if (!confirmed_pass) {
68 rc = -ENOMEM;
69 ecryptfs_syslog(LOG_ERR, "Failed to allocate memory\n");
70- goto out;
71+ goto ret;
72 }
73 mlock(confirmed_pass, ECRYPTFS_MAX_PASSWORD_LENGTH);
74 printf("\n\tMount-wide passphrase: ");
75@@ -229,6 +229,7 @@
76 out:
77 memset(confirmed_pass, 0, ECRYPTFS_MAX_PASSWORD_LENGTH);
78 free(confirmed_pass);
79+ret:
80 return rc;
81 }
82
83
84=== modified file 'src/utils/mount.ecryptfs_private.c'
85--- src/utils/mount.ecryptfs_private.c 2012-12-04 20:00:49 +0000
86+++ src/utils/mount.ecryptfs_private.c 2013-06-07 13:34:31 +0000
87@@ -108,11 +108,13 @@
88 int i;
89 char c;
90 int len;
91+
92+ if (u == NULL)
93+ goto empty;
94 len = strlen(u);
95- if (u == NULL || len == 0) {
96- fputs("Username is empty\n", stderr);
97- return 1;
98- }
99+ if (len == 0)
100+ goto empty;
101+
102 for (i=0; i<len; i++) {
103 c = u[i];
104 if ( !(c>='a' && c<='z') && !(c>='A' && c<='Z') &&
105@@ -126,6 +128,9 @@
106 }
107 }
108 return 0;
109+empty:
110+ fputs("Username is empty\n", stderr);
111+ return 1;
112 }
113
114 char **fetch_sig(char *pw_dir, char *alias, int mounting) {
115@@ -390,8 +395,10 @@
116 (fstat(fd, &s)==0 && (S_ISREG(s.st_mode) && s.st_uid==uid))) {
117 break;
118 } else {
119- if (fd >= 0)
120+ if (fd >= 0) {
121 close(fd);
122+ fd = -1;
123+ }
124 free(f);
125 if (asprintf(&f, "%s/%s-%s-%s-%d", TMP, FSTYPE, u,
126 alias, i++) < 0) {
127
128=== modified file 'tests/kernel/inotify/test.c'
129--- tests/kernel/inotify/test.c 2012-01-31 13:55:17 +0000
130+++ tests/kernel/inotify/test.c 2013-06-07 13:34:31 +0000
131@@ -269,6 +269,7 @@
132 {
133 int fd;
134 char buffer[1];
135+ int rc = 0;
136
137 if ((fd = open(path, O_RDONLY)) < 0) {
138 fprintf(stderr, "Cannot open %s: %s\n", path, strerror(errno));
139@@ -278,9 +279,12 @@
140 /* Just want to force an access */
141 if (read(fd, buffer, 1) < 0) {
142 fprintf(stderr, "Cannot read %s: %s\n", path, strerror(errno));
143- return -1;
144+ rc = -1;
145 }
146- return 0;
147+
148+ close(fd);
149+
150+ return rc;
151 }
152
153 int test_access_file(char *path)
154@@ -301,23 +305,27 @@
155 int test_modify_helper(char *path, void *dummy)
156 {
157 int fd;
158- char buffer[1];
159+ char buffer[1] = { 0 };
160+ int rc = 0;
161
162 if (mk_file(path, 4096) < 0)
163 return -1;
164
165 if ((fd = open(path, O_RDWR)) < 0) {
166 fprintf(stderr, "Cannot open %s: %s\n", path, strerror(errno));
167- return -1;
168+ rc = -1;
169+ goto remove;
170 }
171
172 if (write(fd, buffer, 1) < 0) {
173- fprintf(stderr, "Cannot read %s: %s\n", path, strerror(errno));
174- (void)unlink(path);
175- return -1;
176+ fprintf(stderr, "Cannot write %s: %s\n", path, strerror(errno));
177+ rc = -1;
178 }
179+
180+ close(fd);
181+remove:
182 (void)unlink(path);
183- return 0;
184+ return rc;
185 }
186
187 int test_modify_file(char *path)
188
189=== modified file 'tests/kernel/trunc-file/test.c'
190--- tests/kernel/trunc-file/test.c 2013-03-13 14:55:11 +0000
191+++ tests/kernel/trunc-file/test.c 2013-06-07 13:34:31 +0000
192@@ -84,10 +84,12 @@
193
194 if (write_buff(fd, buff, n) < 0) {
195 close(fd);
196- return TEST_FAILED;
197+ return -1;
198 }
199 buflen -= n;
200 }
201+
202+ return 0;
203 }
204
205 int test_read_random(char *filename, int fd, unsigned char *buff, ssize_t size)
206@@ -253,7 +255,7 @@
207
208 int main(int argc, char **argv)
209 {
210- off_t len = DEFAULT_SIZE;
211+ ssize_t len = DEFAULT_SIZE;
212 int i;
213 int ret;
214
215@@ -278,5 +280,5 @@
216
217 signal(SIGINT, sighandler);
218
219- exit(test_exercise(argv[1], (ssize_t)len));
220+ exit(test_exercise(argv[1], len));
221 }
222
223=== modified file 'tests/userspace/wrap-unwrap/test.c'
224--- tests/userspace/wrap-unwrap/test.c 2012-11-02 23:20:18 +0000
225+++ tests/userspace/wrap-unwrap/test.c 2013-06-07 13:34:31 +0000
226@@ -102,7 +102,7 @@
227 if ((rc = ecryptfs_wrap_passphrase(path, "testwrappw", salt,
228 passphrase)) == 0) {
229 fprintf(stderr, "ecryptfs_wrap_passphrase() returned rc = 0; "
230- "expected error result instead\n", rc);
231+ "expected error result instead\n");
232 rc = 1;
233 goto out;
234 }

Subscribers

People subscribed via source and target branches