From b7343ebb6369fff623c72b0c41270fb0bb7749ec Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Wed, 10 Oct 2018 20:35:17 -0700 Subject: [PATCH] AOSP: android/perms.c: clean up error handling There are a number of error conditions which, due to the way ext2fs_dir_iterate2 operates, would not be propagated to the upper layers of the call stack. As a result, certain error conditions, such as not having enough room to allocate blocks for SELinux labels, would fail silently, instead of causing a compile failure. As suggested in https://android-review.googlesource.com/c/platform/external/e2fsprogs/+/324363 , add a error field to the caller's private data structure, and use the bit in the field to indicate an error condition. Now, certain errors which were silently ignored will cause a compile failure when compiling Android. Test: Artifically modify selabel_lookup() to return a failure, and verify Android doesn't compile. Test: Verify Android compiles under normal circumstances. Test: Artifically modify ino_add_xattr() to return a failure, and verify Android doesn't compile. Bug: 117502873 Bug: 117567573 Bug: 117473440 Signed-off-by: Theodore Ts'o Change-Id: Icdb0105a77e98c3428f20d3c59bf824dcad5db8d From AOSP commit: 7ca13b8b2953f93536ea09eb2ff19bd7cc85b3c1 --- contrib/android/perms.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/contrib/android/perms.c b/contrib/android/perms.c index 746b94b..3c42e59 100644 --- a/contrib/android/perms.c +++ b/contrib/android/perms.c @@ -25,6 +25,7 @@ struct inode_params { time_t fixed_time; const struct ugid_map* uid_map; const struct ugid_map* gid_map; + errcode_t error; }; static errcode_t ino_add_xattr(ext2_filsys fs, ext2_ino_t ino, const char *name, @@ -82,7 +83,7 @@ static errcode_t set_selinux_xattr(ext2_filsys fs, ext2_ino_t ino, if (retval < 0) { com_err(__func__, retval, _("searching for label \"%s\""), params->filename); - exit(1); + return retval; } retval = ino_add_xattr(fs, ino, "security." XATTR_SELINUX_SUFFIX, @@ -249,8 +250,10 @@ static int walk_dir(ext2_ino_t dir EXT2FS_ATTR((unused)), return 0; if (asprintf(¶ms->filename, "%s/%.*s", params->path, name_len, - de->name) < 0) + de->name) < 0) { + params->error = ENOMEM; return -ENOMEM; + } if (!strncmp(de->name, "lost+found", 10)) { retval = set_selinux_xattr(params->fs, de->inode, params); @@ -264,8 +267,10 @@ static int walk_dir(ext2_ino_t dir EXT2FS_ATTR((unused)), char *cur_path = params->path; char *cur_filename = params->filename; params->path = params->filename; - ext2fs_dir_iterate2(params->fs, de->inode, 0, NULL, - walk_dir, params); + retval = ext2fs_dir_iterate2(params->fs, de->inode, 0, NULL, + walk_dir, params); + if (retval) + goto end; params->path = cur_path; params->filename = cur_filename; } @@ -273,6 +278,7 @@ static int walk_dir(ext2_ino_t dir EXT2FS_ATTR((unused)), end: free(params->filename); + params->error |= retval; return retval; } @@ -298,6 +304,7 @@ errcode_t __android_configure_fs(ext2_filsys fs, char *src_dir, .mountpoint = mountpoint, .uid_map = uid_map, .gid_map = gid_map, + .error = 0 }; /* walk_dir will add the "/". Don't add it twice. */ @@ -308,8 +315,11 @@ errcode_t __android_configure_fs(ext2_filsys fs, char *src_dir, if (retval) return retval; - return ext2fs_dir_iterate2(fs, EXT2_ROOT_INO, 0, NULL, walk_dir, - ¶ms); + retval = ext2fs_dir_iterate2(fs, EXT2_ROOT_INO, 0, NULL, walk_dir, + ¶ms); + if (retval) + return retval; + return params.error; } errcode_t android_configure_fs(ext2_filsys fs, char *src_dir, char *target_out, -- 1.8.3.1