From ab90901793d32ff1dd0c9aa7aa7178e4e7e63277 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Sun, 7 Jul 2019 00:31:49 -0700 Subject: [PATCH] Use C++ smart pointer for caching su_info --- native/jni/su/connect.cpp | 42 ++++++++++++------------ native/jni/su/su.h | 43 +++++++++++-------------- native/jni/su/su_daemon.cpp | 64 ++++++++++++++++--------------------- 3 files changed, 67 insertions(+), 82 deletions(-) diff --git a/native/jni/su/connect.cpp b/native/jni/su/connect.cpp index e58e50049..f59155863 100644 --- a/native/jni/su/connect.cpp +++ b/native/jni/su/connect.cpp @@ -10,6 +10,8 @@ #include "su.h" +using namespace std; + bool CONNECT_BROADCAST; #define START_ACTIVITY \ @@ -33,21 +35,21 @@ static inline const char *get_command(const su_request *to) { return DEFAULT_SHELL; } -static inline void get_user(char *user, su_info *info) { +static inline void get_user(char *user, const su_info *info) { sprintf(user, "%d", info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_USER ? info->uid / 100000 : 0); } -static inline void get_uid(char *uid, su_info *info) { +static inline void get_uid(char *uid, const su_info *info) { sprintf(uid, "%d", info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ? info->uid % 100000 : info->uid); } -static void exec_am_cmd(const char **args, su_info *info) { +static void exec_am_cmd(const char **args, const su_info *info) { char component[128]; sprintf(component, "%s/%s", info->str[SU_MANAGER].data(), args[3][0] == 'b' ? "a.h" : "a.m"); char user[8]; @@ -76,29 +78,29 @@ static void exec_am_cmd(const char **args, su_info *info) { "--ei", "to.uid", toUid, \ "--ei", "pid", pid, \ "--ei", "policy", policy, \ -"--es", "command", get_command(&ctx->req), \ -"--ez", "notify", ctx->info->access.notify ? "true" : "false", \ +"--es", "command", get_command(&ctx.req), \ +"--ez", "notify", ctx.info->access.notify ? "true" : "false", \ nullptr -void app_log(su_context *ctx) { +void app_log(const su_context &ctx) { char fromUid[8]; - get_uid(fromUid, ctx->info); + get_uid(fromUid, ctx.info.get()); char toUid[8]; - sprintf(toUid, "%d", ctx->req.uid); + sprintf(toUid, "%d", ctx.req.uid); char pid[8]; - sprintf(pid, "%d", ctx->pid); + sprintf(pid, "%d", ctx.pid); char policy[2]; - sprintf(policy, "%d", ctx->info->access.policy); + sprintf(policy, "%d", ctx.info->access.policy); if (CONNECT_BROADCAST) { const char *cmd[] = { START_BROADCAST, LOG_BODY }; - exec_am_cmd(cmd, ctx->info); + exec_am_cmd(cmd, ctx.info.get()); } else { const char *cmd[] = { START_ACTIVITY, LOG_BODY }; - exec_am_cmd(cmd, ctx->info); + exec_am_cmd(cmd, ctx.info.get()); } } @@ -108,30 +110,30 @@ void app_log(su_context *ctx) { "--ei", "policy", policy, \ nullptr -void app_notify(su_context *ctx) { +void app_notify(const su_context &ctx) { char fromUid[8]; - get_uid(fromUid, ctx->info); + get_uid(fromUid, ctx.info.get()); char policy[2]; - sprintf(policy, "%d", ctx->info->access.policy); + sprintf(policy, "%d", ctx.info->access.policy); if (CONNECT_BROADCAST) { const char *cmd[] = { START_BROADCAST, NOTIFY_BODY }; - exec_am_cmd(cmd, ctx->info); + exec_am_cmd(cmd, ctx.info.get()); } else { const char *cmd[] = { START_ACTIVITY, NOTIFY_BODY }; - exec_am_cmd(cmd, ctx->info); + exec_am_cmd(cmd, ctx.info.get()); } } -void app_connect(const char *socket, su_info *info) { +void app_connect(const char *socket, const shared_ptr &info) { const char *cmd[] = { START_ACTIVITY, "request", "--es", "socket", socket, nullptr }; - exec_am_cmd(cmd, info); + exec_am_cmd(cmd, info.get()); } void broadcast_test() { @@ -144,7 +146,7 @@ void broadcast_test() { exec_am_cmd(cmd, &info); } -void socket_send_request(int fd, su_info *info) { +void socket_send_request(int fd, const shared_ptr &info) { write_key_token(fd, "uid", info->uid); write_string_be(fd, "eof"); } diff --git a/native/jni/su/su.h b/native/jni/su/su.h index 6aa4b1fcd..f78e28b33 100644 --- a/native/jni/su/su.h +++ b/native/jni/su/su.h @@ -1,44 +1,39 @@ -/* su.h - Store all general su info - */ +#pragma once -#ifndef _SU_H_ -#define _SU_H_ - -#include #include #include +#include +#include #include #define DEFAULT_SHELL "/system/bin/sh" // Constants for atty -#define ATTY_IN 1 -#define ATTY_OUT 2 -#define ATTY_ERR 4 +#define ATTY_IN (1 << 0) +#define ATTY_OUT (1 << 1) +#define ATTY_ERR (1 << 2) class su_info { public: - unsigned uid; /* Unique key to find su_info */ - int count; /* Just a count for debugging purpose */ + /* Unique key */ + const unsigned uid; - /* These values should be guarded with internal lock */ + /* These should be guarded with internal lock */ db_settings cfg; db_strings str; su_access access; struct stat mgr_st; - /* These should be guarded with global cache lock */ - int ref; - time_t timestamp; + /* This should be guarded with global cache lock */ + long timestamp; su_info(unsigned uid = 0); ~su_info(); void lock(); void unlock(); - bool isFresh(); - void newRef(); - void deRef(); + bool is_fresh(); + void refresh(); private: pthread_mutex_t _lock; /* Internal lock */ @@ -60,16 +55,14 @@ struct su_request : public su_req_base { } __attribute__((packed)); struct su_context { - su_info *info; + std::shared_ptr info; su_request req; pid_t pid; }; // connect.c -void app_log(su_context *ctx); -void app_notify(su_context *ctx); -void app_connect(const char *socket, su_info *info); -void socket_send_request(int fd, su_info *info); - -#endif +void app_log(const su_context &ctx); +void app_notify(const su_context &ctx); +void app_connect(const char *socket, const std::shared_ptr &info); +void socket_send_request(int fd, const std::shared_ptr &info); diff --git a/native/jni/su/su_daemon.cpp b/native/jni/su/su_daemon.cpp index c498ab2f8..3ccea7aa1 100644 --- a/native/jni/su/su_daemon.cpp +++ b/native/jni/su/su_daemon.cpp @@ -20,14 +20,16 @@ #include "su.h" #include "pts.h" +using namespace std; + #define LOCK_CACHE() pthread_mutex_lock(&cache_lock) #define UNLOCK_CACHE() pthread_mutex_unlock(&cache_lock) static pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER; -static su_info *cache; +static shared_ptr cached; su_info::su_info(unsigned uid) : - uid(uid), count(0), access(DEFAULT_SU_ACCESS), mgr_st({}), ref(0), + uid(uid), access(DEFAULT_SU_ACCESS), mgr_st({}), timestamp(0), _lock(PTHREAD_MUTEX_INITIALIZER) {} su_info::~su_info() { @@ -42,27 +44,20 @@ void su_info::unlock() { pthread_mutex_unlock(&_lock); } -bool su_info::isFresh() { - return time(nullptr) - timestamp < 3; /* 3 seconds */ +bool su_info::is_fresh() { + timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + long current = ts.tv_sec * 1000L + ts.tv_nsec / 1000L; + return current - timestamp < 3000; /* 3 seconds */ } -void su_info::newRef() { - timestamp = time(nullptr); - ++ref; +void su_info::refresh() { + timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + timestamp = ts.tv_sec * 1000L + ts.tv_nsec / 1000L; } -void su_info::deRef() { - LOCK_CACHE(); - --ref; - if (ref == 0 && !isFresh()) { - if (cache == this) - cache = nullptr; - delete this; - } - UNLOCK_CACHE(); -} - -static void database_check(su_info *info) { +static void database_check(const shared_ptr &info) { int uid = info->uid; get_db_settings(info->cfg); get_db_strings(info->str); @@ -91,28 +86,24 @@ static void database_check(su_info *info) { validate_manager(info->str[SU_MANAGER], uid / 100000, &info->mgr_st); } -static su_info *get_su_info(unsigned uid) { - su_info *info = nullptr; +static shared_ptr get_su_info(unsigned uid) { + shared_ptr info; // Get from cache or new instance LOCK_CACHE(); - if (cache && cache->uid == uid && cache->isFresh()) { - info = cache; - } else { - if (cache && cache->ref == 0) - delete cache; - cache = info = new su_info(uid); - } - info->newRef(); + if (!cached || cached->uid != uid || !cached->is_fresh()) + cached = make_shared(uid); + info = cached; + info->refresh(); UNLOCK_CACHE(); - LOGD("su: request from uid=[%d] (#%d)\n", info->uid, ++info->count); + LOGD("su: request from uid=[%d]\n", info->uid); // Lock before the policy is determined info->lock(); if (info->access.policy == QUERY) { - // Not cached, get data from database + // Not cached, get data from database database_check(info); // Check su access settings @@ -195,13 +186,12 @@ static void set_identity(unsigned uid) { void su_daemon_handler(int client, struct ucred *credential) { LOGD("su: request from pid=[%d], client=[%d]\n", credential->pid, client); - - su_info *info = get_su_info(credential->uid); + + auto info = get_su_info(credential->uid); // Fail fast if (info->access.policy == DENY && info->str[SU_MANAGER][0] == '\0') { LOGD("su: fast deny\n"); - info->deRef(); write_int(client, DENY); close(client); return; @@ -214,7 +204,7 @@ void su_daemon_handler(int client, struct ucred *credential) { */ int child = xfork(); if (child) { - info->deRef(); + info.reset(); // Wait result LOGD("su: waiting child pid=[%d]\n", child); @@ -320,9 +310,9 @@ void su_daemon_handler(int client, struct ucred *credential) { } if (info->access.log) - app_log(&ctx); + app_log(ctx); else if (info->access.notify) - app_notify(&ctx); + app_notify(ctx); if (info->access.policy == ALLOW) { const char *argv[] = { nullptr, nullptr, nullptr, nullptr };