tequilaOS/platform_system_core
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix recursive locking bug.

Browse source 
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
This commit is contained in:
Jeff Sharkey 2013-08-30 10:26:15 -07:00
parent 7e18e45617
commit aa04e818a4
1 changed files with 36 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
sdcard/sdcard.c
View file

@ -486,12 +486,18 @@ static void derive_permissions_locked(struct fuse* fuse, struct node *parent,
}
}
/* Return if the calling UID holds sdcard_rw. */
static bool get_caller_has_rw_locked(struct fuse* fuse, const struct fuse_in_header *hdr) {
appid_t appid = multiuser_get_app_id(hdr->uid);
return hashmapContainsKey(fuse->appid_with_rw, (void*) appid);
}
/* Kernel has already enforced everything we returned through
* derive_permissions_locked(), so this is used to lock down access
* even further, such as enforcing that apps hold sdcard_rw. */
static bool check_caller_access_to_name(struct fuse* fuse,
const struct fuse_in_header *hdr, const struct node* parent_node,
const char* name, int mode) {
const char* name, int mode, bool has_rw) {
/* Always block security-sensitive files at root */
if (parent_node && parent_node->perm == PERM_ROOT) {
if (!strcmp(name, "autorun.inf")
@ -518,13 +524,7 @@ static bool check_caller_access_to_name(struct fuse* fuse,
return true;
}
appid_t appid = multiuser_get_app_id(hdr->uid);
pthread_mutex_lock(&fuse->lock);
bool hasRw = hashmapContainsKey(fuse->appid_with_rw, (void*) appid);
pthread_mutex_unlock(&fuse->lock);
return hasRw;
return has_rw;
}
/* No extra permissions to enforce */
@ -532,8 +532,8 @@ static bool check_caller_access_to_name(struct fuse* fuse,
}
static bool check_caller_access_to_node(struct fuse* fuse,
const struct fuse_in_header *hdr, const struct node* node, int mode) {
return check_caller_access_to_name(fuse, hdr, node->parent, node->name, mode);
const struct fuse_in_header *hdr, const struct node* node, int mode, bool has_rw) {
return check_caller_access_to_name(fuse, hdr, node->parent, node->name, mode, has_rw);
}
struct node *create_node_locked(struct fuse* fuse,
@ -831,7 +831,7 @@ static int handle_lookup(struct fuse* fuse, struct fuse_handler* handler,
child_path, sizeof(child_path), 1))) {
return -ENOENT;
}
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, R_OK)) {
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, R_OK, false)) {
return -EACCES;
}
@ -872,7 +872,7 @@ static int handle_getattr(struct fuse* fuse, struct fuse_handler* handler,
if (!node) {
return -ENOENT;
}
if (!check_caller_access_to_node(fuse, hdr, node, R_OK)) {
if (!check_caller_access_to_node(fuse, hdr, node, R_OK, false)) {
return -EACCES;
}
@ -882,11 +882,13 @@ static int handle_getattr(struct fuse* fuse, struct fuse_handler* handler,
static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header *hdr, const struct fuse_setattr_in *req)
{
bool has_rw;
struct node* node;
char path[PATH_MAX];
struct timespec times[2];
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path));
TRACE("[%d] SETATTR fh=%llx valid=%x @ %llx (%s)\n", handler->token,
req->fh, req->valid, hdr->nodeid, node ? node->name : "?");
@ -895,7 +897,7 @@ static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler,
if (!node) {
return -ENOENT;
}
if (!check_caller_access_to_node(fuse, hdr, node, W_OK)) {
if (!check_caller_access_to_node(fuse, hdr, node, W_OK, has_rw)) {
return -EACCES;
}
@ -943,12 +945,14 @@ static int handle_setattr(struct fuse* fuse, struct fuse_handler* handler,
static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const struct fuse_mknod_in* req, const char* name)
{
bool has_rw;
struct node* parent_node;
char parent_path[PATH_MAX];
char child_path[PATH_MAX];
const char* actual_name;
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid,
parent_path, sizeof(parent_path));
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,
child_path, sizeof(child_path), 1))) {
return -ENOENT;
}
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) {
return -EACCES;
}
__u32 mode = (req->mode & (~0777)) | 0664;
@ -972,12 +976,14 @@ static int handle_mknod(struct fuse* fuse, struct fuse_handler* handler,
static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const struct fuse_mkdir_in* req, const char* name)
{
bool has_rw;
struct node* parent_node;
char parent_path[PATH_MAX];
char child_path[PATH_MAX];
const char* actual_name;
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid,
parent_path, sizeof(parent_path));
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,
child_path, sizeof(child_path), 1))) {
return -ENOENT;
}
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) {
return -EACCES;
}
__u32 mode = (req->mode & (~0777)) | 0775;
@ -1001,11 +1007,13 @@ static int handle_mkdir(struct fuse* fuse, struct fuse_handler* handler,
static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const char* name)
{
bool has_rw;
struct node* parent_node;
char parent_path[PATH_MAX];
char child_path[PATH_MAX];
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid,
parent_path, sizeof(parent_path));
TRACE("[%d] UNLINK %s @ %llx (%s)\n", handler->token,
@ -1016,7 +1024,7 @@ static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler,
child_path, sizeof(child_path), 1)) {
return -ENOENT;
}
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) {
return -EACCES;
}
if (unlink(child_path) < 0) {
@ -1028,11 +1036,13 @@ static int handle_unlink(struct fuse* fuse, struct fuse_handler* handler,
static int handle_rmdir(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const char* name)
{
bool has_rw;
struct node* parent_node;
char parent_path[PATH_MAX];
char child_path[PATH_MAX];
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid,
parent_path, sizeof(parent_path));
TRACE("[%d] RMDIR %s @ %llx (%s)\n", handler->token,
@ -1043,7 +1053,7 @@ static int handle_rmdir(struct fuse* fuse, struct fuse_handler* handler,
child_path, sizeof(child_path), 1)) {
return -ENOENT;
}
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, parent_node, name, W_OK, has_rw)) {
return -EACCES;
}
if (rmdir(child_path) < 0) {
@ -1056,6 +1066,7 @@ static int handle_rename(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const struct fuse_rename_in* req,
const char* old_name, const char* new_name)
{
bool has_rw;
struct node* old_parent_node;
struct node* new_parent_node;
struct node* child_node;
@ -1067,6 +1078,7 @@ static int handle_rename(struct fuse* fuse, struct fuse_handler* handler,
int res;
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
old_parent_node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid,
old_parent_path, sizeof(old_parent_path));
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,
res = -ENOENT;
goto lookup_error;
}
if (!check_caller_access_to_name(fuse, hdr, old_parent_node, old_name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, old_parent_node, old_name, W_OK, has_rw)) {
res = -EACCES;
goto lookup_error;
}
if (!check_caller_access_to_name(fuse, hdr, new_parent_node, new_name, W_OK)) {
if (!check_caller_access_to_name(fuse, hdr, new_parent_node, new_name, W_OK, has_rw)) {
res = -EACCES;
goto lookup_error;
}
@ -1146,12 +1158,14 @@ static int open_flags_to_access_mode(int open_flags) {
static int handle_open(struct fuse* fuse, struct fuse_handler* handler,
const struct fuse_in_header* hdr, const struct fuse_open_in* req)
{
bool has_rw;
struct node* node;
char path[PATH_MAX];
struct fuse_open_out out;
struct handle *h;
pthread_mutex_lock(&fuse->lock);
has_rw = get_caller_has_rw_locked(fuse, hdr);
node = lookup_node_and_path_by_id_locked(fuse, hdr->nodeid, path, sizeof(path));
TRACE("[%d] OPEN 0%o @ %llx (%s)\n", handler->token,
req->flags, hdr->nodeid, node ? node->name : "?");
@ -1160,7 +1174,8 @@ static int handle_open(struct fuse* fuse, struct fuse_handler* handler,
if (!node) {
return -ENOENT;
}
if (!check_caller_access_to_node(fuse, hdr, node, open_flags_to_access_mode(req->flags))) {
if (!check_caller_access_to_node(fuse, hdr, node,
open_flags_to_access_mode(req->flags), has_rw)) {
return -EACCES;
}
h = malloc(sizeof(*h));
@ -1307,7 +1322,7 @@ static int handle_opendir(struct fuse* fuse, struct fuse_handler* handler,
if (!node) {
return -ENOENT;
}
if (!check_caller_access_to_node(fuse, hdr, node, R_OK)) {
if (!check_caller_access_to_node(fuse, hdr, node, R_OK, false)) {
return -EACCES;
}
h = malloc(sizeof(*h));