diff options
author | Jeff Sharkey | 2013-08-30 12:26:15 -0500 |
---|---|---|
committer | The Android Automerger | 2013-08-30 16:13:23 -0500 |
commit | ec0639a58a8c018dd8742762f1c4899e013ffc65 (patch) | |
tree | 603f13c762828a694f1f9c31f3837a96b4edf5ca | |
parent | 5f39562466688da2f6a0d6fc1588e8a3f5c54971 (diff) | |
download | platform-system-core-ec0639a58a8c018dd8742762f1c4899e013ffc65.tar.gz platform-system-core-ec0639a58a8c018dd8742762f1c4899e013ffc65.tar.xz platform-system-core-ec0639a58a8c018dd8742762f1c4899e013ffc65.zip |
Fix recursive locking bug.
handle_rename() would end up acquiring the lock twice. Change to
always derive has_rw inside earlier locks (instead of acquiring a
second time), and pass the value into check_caller_access_to_name().
Bug: 10547597
Change-Id: If5744d6d226a4785676c19d0f7fdf1c05060ed76
-rw-r--r-- | sdcard/sdcard.c | 57 |
1 files changed, 36 insertions, 21 deletions
diff --git a/sdcard/sdcard.c b/sdcard/sdcard.c index 7349b2241..330d555b5 100644 --- a/sdcard/sdcard.c +++ b/sdcard/sdcard.c | |||
@@ -486,12 +486,18 @@ static void derive_permissions_locked(struct fuse* fuse, struct node *parent, | |||
486 | } | 486 | } |
487 | } | 487 | } |
488 | 488 | ||
489 | /* Return if the calling UID holds sdcard_rw. */ | ||
490 | static bool get_caller_has_rw_locked(struct fuse* fuse, const struct fuse_in_header *hdr) { | ||
491 | appid_t appid = multiuser_get_app_id(hdr->uid); | ||
492 | return hashmapContainsKey(fuse->appid_with_rw, (void*) appid); | ||
493 | } | ||
494 | |||
489 | /* Kernel has already enforced everything we returned through | 495 | /* Kernel has already enforced everything we returned through |
490 | * derive_permissions_locked(), so this is used to lock down access | 496 | * derive_permissions_locked(), so this is used to lock down access |
491 | * even further, such as enforcing that apps hold sdcard_rw. */ | 497 | * even further, such as enforcing that apps hold sdcard_rw. */ |
492 | static bool check_caller_access_to_name(struct fuse* fuse, | 498 | static bool check_caller_access_to_name(struct fuse* fuse, |
493 | const struct fuse_in_header *hdr, const struct node* parent_node, | 499 | const struct fuse_in_header *hdr, const struct node* parent_node, |
494 | const char* name, int mode) { | 500 | const char* name, int mode, bool has_rw) { |
495 | /* Always block security-sensitive files at root */ | 501 | /* Always block security-sensitive files at root */ |
496 | if (parent_node && parent_node->perm == PERM_ROOT) { | 502 | if (parent_node && parent_node->perm == PERM_ROOT) { |
497 | if (!strcmp(name, "autorun.inf") | 503 | if (!strcmp(name, "autorun.inf") |
@@ -518,13 +524,7 @@ static bool check_caller_access_to_name(struct fuse* fuse, | |||
518 | return true; | 524 | return true; |
519 | } | 525 | } |
520 | 526 | ||
521 | appid_t appid = multiuser_get_app_id(hdr->uid); | 527 | return has_rw; |
522 | |||
523 | pthread_mutex_lock(&fuse->lock); | ||
524 | bool hasRw = hashmapContainsKey(fuse->appid_with_rw, (void*) appid); | ||
525 | pthread_mutex_unlock(&fuse->lock); | ||
526 | |||
527 | return hasRw; | ||
528 | } | 528 | } |
529 | 529 | ||
530 | /* No extra permissions to enforce */ | 530 | /* No extra permissions to enforce */ |
@@ -532,8 +532,8 @@ static bool check_caller_access_to_name(struct fuse* fuse, | |||
532 | } | 532 | } |
533 | 533 | ||
534 | static bool check_caller_access_to_node(struct fuse* fuse, | 534 | static bool check_caller_access_to_node(struct fuse* fuse, |
535 | const struct fuse_in_header *hdr, const struct node* node, int mode) { | 535 | const struct fuse_in_header *hdr, const struct node* node, int mode, bool has_rw) { |
536 | return check_caller_access_to_name(fuse, hdr, node->parent, node->name, mode); | 536 | return check_caller_access_to_name(fuse, hdr, node->parent, node->name, mode, has_rw); |
537 | } | 537 | } |
538 | 538 | ||
539 | struct node *create_node_locked(struct fuse* fuse, | 539 | struct node *create_node_locked(struct fuse* fuse, |
@@ -831,7 +831,7 @@ static int handle_lookup(struct fuse* fuse, struct fuse_handler* handler, | |||
831 | child_path, sizeof(child_path), 1))) { | 831 | child_path, sizeof(child_path), 1))) { |
832 | return -ENOENT; | 832 | return -ENOENT; |
833 | } | 833 | } |
834 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, R_OK)) { | 834 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, R_OK, false)) { |
835 | return -EACCES; | 835 | return -EACCES; |
836 | } | 836 | } |
837 | 837 | ||
@@ -872,7 +872,7 @@ static int handle_getattr(struct fuse* fuse, struct fuse_handler* handler, | |||
872 | if (!node) { | 872 | if (!node) { |
873 | return -ENOENT; | 873 | return -ENOENT; |
874 | } | 874 | } |
875 | if (!check_caller_access_to_node(fuse, hdr, node, R_OK)) { | 875 | if (!check_caller_access_to_node(fuse, hdr, node, R_OK, false)) { |
876 | return -EACCES; | 876 | return -EACCES; |
877 | } | 877 | } |
878 | 878 | ||
@@ -882,11 +882,13 @@ static int handle_getattr(struct fuse* fuse, struct fuse_handler* handler, | |||
882 | static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler, | 882 | static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler, |
883 | const struct fuse_in_header *hdr, const struct fuse_setattr_in *req) | 883 | const struct fuse_in_header *hdr, const struct fuse_setattr_in *req) |
884 | { | 884 | { |
885 | bool has_rw; | ||
885 | struct node* node; | 886 | struct node* node; |
886 | char path[PATH_MAX]; | 887 | char path[PATH_MAX]; |
887 | struct timespec times[2]; | 888 | struct timespec times[2]; |
888 | 889 | ||
889 | pthread_mutex_lock(&fuse->lock); | 890 | pthread_mutex_lock(&fuse->lock); |
891 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
890 | node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path)); | 892 | node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path)); |
891 | TRACE("[%d] SETATTR fh=%llx valid=%x @ %llx (%s)\n", handler->token, | 893 | TRACE("[%d] SETATTR fh=%llx valid=%x @ %llx (%s)\n", handler->token, |
892 | req->fh, req->valid, hdr->nodeid, node ? node->name : "?"); | 894 | req->fh, req->valid, hdr->nodeid, node ? node->name : "?"); |
@@ -895,7 +897,7 @@ static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler, | |||
895 | if (!node) { | 897 | if (!node) { |
896 | return -ENOENT; | 898 | return -ENOENT; |
897 | } | 899 | } |
898 | if (!check_caller_access_to_node(fuse, hdr, node, W_OK)) { | 900 | if (!check_caller_access_to_node(fuse, hdr, node, W_OK, has_rw)) { |
899 | return -EACCES; | 901 | return -EACCES; |
900 | } | 902 | } |
901 | 903 | ||
@@ -943,12 +945,14 @@ static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler, | |||
943 | static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler, | 945 | static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler, |
944 | const struct fuse_in_header* hdr, const struct fuse_mknod_in* req, const char* name) | 946 | const struct fuse_in_header* hdr, const struct fuse_mknod_in* req, const char* name) |
945 | { | 947 | { |
948 | bool has_rw; | ||
946 | struct node* parent_node; | 949 | struct node* parent_node; |
947 | char parent_path[PATH_MAX]; | 950 | char parent_path[PATH_MAX]; |
948 | char child_path[PATH_MAX]; | 951 | char child_path[PATH_MAX]; |
949 | const char* actual_name; | 952 | const char* actual_name; |
950 | 953 | ||
951 | pthread_mutex_lock(&fuse->lock); | 954 | pthread_mutex_lock(&fuse->lock); |
955 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
952 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, | 956 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, |
953 | parent_path, sizeof(parent_path)); | 957 | parent_path, sizeof(parent_path)); |
954 | TRACE("[%d] MKNOD %s 0%o @ %llx (%s)\n", handler->token, | 958 | TRACE("[%d] MKNOD %s 0%o @ %llx (%s)\n", handler->token, |
@@ -959,7 +963,7 @@ static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler, | |||
959 | child_path, sizeof(child_path), 1))) { | 963 | child_path, sizeof(child_path), 1))) { |
960 | return -ENOENT; | 964 | return -ENOENT; |
961 | } | 965 | } |
962 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) { | 966 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) { |
963 | return -EACCES; | 967 | return -EACCES; |
964 | } | 968 | } |
965 | __u32 mode = (req->mode & (~0777)) | 0664; | 969 | __u32 mode = (req->mode & (~0777)) | 0664; |
@@ -972,12 +976,14 @@ static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler, | |||
972 | static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler, | 976 | static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler, |
973 | const struct fuse_in_header* hdr, const struct fuse_mkdir_in* req, const char* name) | 977 | const struct fuse_in_header* hdr, const struct fuse_mkdir_in* req, const char* name) |
974 | { | 978 | { |
979 | bool has_rw; | ||
975 | struct node* parent_node; | 980 | struct node* parent_node; |
976 | char parent_path[PATH_MAX]; | 981 | char parent_path[PATH_MAX]; |
977 | char child_path[PATH_MAX]; | 982 | char child_path[PATH_MAX]; |
978 | const char* actual_name; | 983 | const char* actual_name; |
979 | 984 | ||
980 | pthread_mutex_lock(&fuse->lock); | 985 | pthread_mutex_lock(&fuse->lock); |
986 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
981 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, | 987 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, |
982 | parent_path, sizeof(parent_path)); | 988 | parent_path, sizeof(parent_path)); |
983 | TRACE("[%d] MKDIR %s 0%o @ %llx (%s)\n", handler->token, | 989 | TRACE("[%d] MKDIR %s 0%o @ %llx (%s)\n", handler->token, |
@@ -988,7 +994,7 @@ static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler, | |||
988 | child_path, sizeof(child_path), 1))) { | 994 | child_path, sizeof(child_path), 1))) { |
989 | return -ENOENT; | 995 | return -ENOENT; |
990 | } | 996 | } |
991 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) { | 997 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) { |
992 | return -EACCES; | 998 | return -EACCES; |
993 | } | 999 | } |
994 | __u32 mode = (req->mode & (~0777)) | 0775; | 1000 | __u32 mode = (req->mode & (~0777)) | 0775; |
@@ -1001,11 +1007,13 @@ static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler, | |||
1001 | static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler, | 1007 | static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler, |
1002 | const struct fuse_in_header* hdr, const char* name) | 1008 | const struct fuse_in_header* hdr, const char* name) |
1003 | { | 1009 | { |
1010 | bool has_rw; | ||
1004 | struct node* parent_node; | 1011 | struct node* parent_node; |
1005 | char parent_path[PATH_MAX]; | 1012 | char parent_path[PATH_MAX]; |
1006 | char child_path[PATH_MAX]; | 1013 | char child_path[PATH_MAX]; |
1007 | 1014 | ||
1008 | pthread_mutex_lock(&fuse->lock); | 1015 | pthread_mutex_lock(&fuse->lock); |
1016 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
1009 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, | 1017 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, |
1010 | parent_path, sizeof(parent_path)); | 1018 | parent_path, sizeof(parent_path)); |
1011 | TRACE("[%d] UNLINK %s @ %llx (%s)\n", handler->token, | 1019 | TRACE("[%d] UNLINK %s @ %llx (%s)\n", handler->token, |
@@ -1016,7 +1024,7 @@ static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler, | |||
1016 | child_path, sizeof(child_path), 1)) { | 1024 | child_path, sizeof(child_path), 1)) { |
1017 | return -ENOENT; | 1025 | return -ENOENT; |
1018 | } | 1026 | } |
1019 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) { | 1027 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) { |
1020 | return -EACCES; | 1028 | return -EACCES; |
1021 | } | 1029 | } |
1022 | if (unlink(child_path) < 0) { | 1030 | if (unlink(child_path) < 0) { |
@@ -1028,11 +1036,13 @@ static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler, | |||
1028 | static int handle_rmdir(struct fuse* fuse, struct fuse_handler* handler, | 1036 | static int handle_rmdir(struct fuse* fuse, struct fuse_handler* handler, |
1029 | const struct fuse_in_header* hdr, const char* name) | 1037 | const struct fuse_in_header* hdr, const char* name) |
1030 | { | 1038 | { |
1039 | bool has_rw; | ||
1031 | struct node* parent_node; | 1040 | struct node* parent_node; |
1032 | char parent_path[PATH_MAX]; | 1041 | char parent_path[PATH_MAX]; |
1033 | char child_path[PATH_MAX]; | 1042 | char child_path[PATH_MAX]; |
1034 | 1043 | ||
1035 | pthread_mutex_lock(&fuse->lock); | 1044 | pthread_mutex_lock(&fuse->lock); |
1045 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
1036 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, | 1046 | parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, |
1037 | parent_path, sizeof(parent_path)); | 1047 | parent_path, sizeof(parent_path)); |
1038 | TRACE("[%d] RMDIR %s @ %llx (%s)\n", handler->token, | 1048 | TRACE("[%d] RMDIR %s @ %llx (%s)\n", handler->token, |
@@ -1043,7 +1053,7 @@ static int handle_rmdir(struct fuse* fuse, struct fuse_handler* handler, | |||
1043 | child_path, sizeof(child_path), 1)) { | 1053 | child_path, sizeof(child_path), 1)) { |
1044 | return -ENOENT; | 1054 | return -ENOENT; |
1045 | } | 1055 | } |
1046 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) { | 1056 | if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) { |
1047 | return -EACCES; | 1057 | return -EACCES; |
1048 | } | 1058 | } |
1049 | if (rmdir(child_path) < 0) { | 1059 | if (rmdir(child_path) < 0) { |
@@ -1056,6 +1066,7 @@ static int handle_rename(struct fuse* fuse, struct fuse_handler* handler, | |||
1056 | const struct fuse_in_header* hdr, const struct fuse_rename_in* req, | 1066 | const struct fuse_in_header* hdr, const struct fuse_rename_in* req, |
1057 | const char* old_name, const char* new_name) | 1067 | const char* old_name, const char* new_name) |
1058 | { | 1068 | { |
1069 | bool has_rw; | ||
1059 | struct node* old_parent_node; | 1070 | struct node* old_parent_node; |
1060 | struct node* new_parent_node; | 1071 | struct node* new_parent_node; |
1061 | struct node* child_node; | 1072 | struct node* child_node; |
@@ -1067,6 +1078,7 @@ static int handle_rename(struct fuse* fuse, struct fuse_handler* handler, | |||
1067 | int res; | 1078 | int res; |
1068 | 1079 | ||
1069 | pthread_mutex_lock(&fuse->lock); | 1080 | pthread_mutex_lock(&fuse->lock); |
1081 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
1070 | old_parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, | 1082 | old_parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, |
1071 | old_parent_path, sizeof(old_parent_path)); | 1083 | old_parent_path, sizeof(old_parent_path)); |
1072 | new_parent_node = lookup_node_and_path_by_id_locked(fuse, req->newdir, | 1084 | new_parent_node = lookup_node_and_path_by_id_locked(fuse, req->newdir, |
@@ -1079,11 +1091,11 @@ static int handle_rename(struct fuse* fuse, struct fuse_handler* handler, | |||
1079 | res = -ENOENT; | 1091 | res = -ENOENT; |
1080 | goto lookup_error; | 1092 | goto lookup_error; |
1081 | } | 1093 | } |
1082 | if (!check_caller_access_to_name(fuse, hdr, old_parent_node, old_name, W_OK)) { | 1094 | if (!check_caller_access_to_name(fuse, hdr, old_parent_node, old_name, W_OK, has_rw)) { |
1083 | res = -EACCES; | 1095 | res = -EACCES; |
1084 | goto lookup_error; | 1096 | goto lookup_error; |
1085 | } | 1097 | } |
1086 | if (!check_caller_access_to_name(fuse, hdr, new_parent_node, new_name, W_OK)) { | 1098 | if (!check_caller_access_to_name(fuse, hdr, new_parent_node, new_name, W_OK, has_rw)) { |
1087 | res = -EACCES; | 1099 | res = -EACCES; |
1088 | goto lookup_error; | 1100 | goto lookup_error; |
1089 | } | 1101 | } |
@@ -1146,12 +1158,14 @@ static int open_flags_to_access_mode(int open_flags) { | |||
1146 | static int handle_open(struct fuse* fuse, struct fuse_handler* handler, | 1158 | static int handle_open(struct fuse* fuse, struct fuse_handler* handler, |
1147 | const struct fuse_in_header* hdr, const struct fuse_open_in* req) | 1159 | const struct fuse_in_header* hdr, const struct fuse_open_in* req) |
1148 | { | 1160 | { |
1161 | bool has_rw; | ||
1149 | struct node* node; | 1162 | struct node* node; |
1150 | char path[PATH_MAX]; | 1163 | char path[PATH_MAX]; |
1151 | struct fuse_open_out out; | 1164 | struct fuse_open_out out; |
1152 | struct handle *h; | 1165 | struct handle *h; |
1153 | 1166 | ||
1154 | pthread_mutex_lock(&fuse->lock); | 1167 | pthread_mutex_lock(&fuse->lock); |
1168 | has_rw = get_caller_has_rw_locked(fuse, hdr); | ||
1155 | node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path)); | 1169 | node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path)); |
1156 | TRACE("[%d] OPEN 0%o @ %llx (%s)\n", handler->token, | 1170 | TRACE("[%d] OPEN 0%o @ %llx (%s)\n", handler->token, |
1157 | req->flags, hdr->nodeid, node ? node->name : "?"); | 1171 | req->flags, hdr->nodeid, node ? node->name : "?"); |
@@ -1160,7 +1174,8 @@ static int handle_open(struct fuse* fuse, struct fuse_handler* handler, | |||
1160 | if (!node) { | 1174 | if (!node) { |
1161 | return -ENOENT; | 1175 | return -ENOENT; |
1162 | } | 1176 | } |
1163 | if (!check_caller_access_to_node(fuse, hdr, node, open_flags_to_access_mode(req->flags))) { | 1177 | if (!check_caller_access_to_node(fuse, hdr, node, |
1178 | open_flags_to_access_mode(req->flags), has_rw)) { | ||
1164 | return -EACCES; | 1179 | return -EACCES; |
1165 | } | 1180 | } |
1166 | h = malloc(sizeof(*h)); | 1181 | h = malloc(sizeof(*h)); |
@@ -1307,7 +1322,7 @@ static int handle_opendir(struct fuse* fuse, struct fuse_handler* handler, | |||
1307 | if (!node) { | 1322 | if (!node) { |
1308 | return -ENOENT; | 1323 | return -ENOENT; |
1309 | } | 1324 | } |
1310 | if (!check_caller_access_to_node(fuse, hdr, node, R_OK)) { | 1325 | if (!check_caller_access_to_node(fuse, hdr, node, R_OK, false)) { |
1311 | return -EACCES; | 1326 | return -EACCES; |
1312 | } | 1327 | } |
1313 | h = malloc(sizeof(*h)); | 1328 | h = malloc(sizeof(*h)); |