Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 69d47aa8 authored by Tom Cherry's avatar Tom Cherry
Browse files

Clean up property set error handling 

Currently we only report why a property set call has failed but drop
the context of what was trying to set the property.  This change
adds information about why a property was trying to be set when it
fails.

It also unifies property_set() within init to go through the same
HandlePropertySet() function as normal processes do, removing unneeded
special cases.

Test: boot bullhead
Test: attempt to set invalid properties and see better error messages
Change-Id: I5cd3a40086fd3b226e9c8a5e3a84cb3b31399c0d
parent 45ee43b1

init/host_init_stubs.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -43,8 +43,8 @@ std::string default_console = "/dev/console"; 




// property_service.h

// property_service.h

uint32_t (*property_set)(const std::string& name, const std::string& value) = nullptr;

uint32_t (*property_set)(const std::string& name, const std::string& value) = nullptr;

uint32_t HandlePropertySet(const std::string&, const std::string&, const std::string&,

uint32_t HandlePropertySet(const std::string&, const std::string&, const std::string&, const ucred&,

                           const ucred&) {

                           std::string*) {

    return 0;

    return 0;

}

}

init/host_init_stubs.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -48,7 +48,7 @@ extern std::string default_console; 
// property_service.h

// property_service.h

extern uint32_t (*property_set)(const std::string& name, const std::string& value);

extern uint32_t (*property_set)(const std::string& name, const std::string& value);

uint32_t HandlePropertySet(const std::string& name, const std::string& value,

uint32_t HandlePropertySet(const std::string& name, const std::string& value,

                           const std::string& source_context, const ucred& cr);

                           const std::string& source_context, const ucred& cr, std::string* error);





// selinux.h

// selinux.h

void SelabelInitialize();

void SelabelInitialize();

init/property_service.cpp

+55 −42
Original line number Original line Diff line number Diff line
@@ -117,23 +117,21 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, 
    return has_access;

    return has_access;

}

}





static uint32_t PropertySetImpl(const std::string& name, const std::string& value) {

static uint32_t PropertySet(const std::string& name, const std::string& value, std::string* error) {

    size_t valuelen = value.size();

    size_t valuelen = value.size();





    if (!IsLegalPropertyName(name)) {

    if (!IsLegalPropertyName(name)) {

        LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: bad name";

        *error = "Illegal property name";

        return PROP_ERROR_INVALID_NAME;

        return PROP_ERROR_INVALID_NAME;

    }

    }





    if (valuelen >= PROP_VALUE_MAX && !StartsWith(name, "ro.")) {

    if (valuelen >= PROP_VALUE_MAX && !StartsWith(name, "ro.")) {

        LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "

        *error = "Property value too long";

                   << "value too long";

        return PROP_ERROR_INVALID_VALUE;

        return PROP_ERROR_INVALID_VALUE;

    }

    }





    if (mbstowcs(nullptr, value.data(), 0) == static_cast<std::size_t>(-1)) {

    if (mbstowcs(nullptr, value.data(), 0) == static_cast<std::size_t>(-1)) {

        LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "

        *error = "Value is not a UTF8 encoded string";

                   << "value not a UTF8 encoded string";

        return PROP_ERROR_INVALID_VALUE;

        return PROP_ERROR_INVALID_VALUE;

    }

    }
@@ -141,8 +139,7 @@ static uint32_t PropertySetImpl(const std::string& name, const std::string& valu 
    if (pi != nullptr) {

    if (pi != nullptr) {

        // ro.* properties are actually "write-once".

        // ro.* properties are actually "write-once".

        if (StartsWith(name, "ro.")) {

        if (StartsWith(name, "ro.")) {

            LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "

            *error = "Read-only property was already set";

                       << "property already set";

            return PROP_ERROR_READ_ONLY_PROPERTY;

            return PROP_ERROR_READ_ONLY_PROPERTY;

        }

        }
@@ -150,8 +147,7 @@ static uint32_t PropertySetImpl(const std::string& name, const std::string& valu 
    } else {

    } else {

        int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen);

        int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen);

        if (rc < 0) {

        if (rc < 0) {

            LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "

            *error = "__system_property_add failed";

                       << "__system_property_add failed";

            return PROP_ERROR_SET_FAILED;

            return PROP_ERROR_SET_FAILED;

        }

        }

    }

    }
@@ -205,8 +201,10 @@ bool PropertyChildReap(pid_t pid) { 
    if (info.pid != pid) {

    if (info.pid != pid) {

        return false;

        return false;

    }

    }

    if (PropertySetImpl(info.name, info.value) != PROP_SUCCESS) {

    std::string error;

        LOG(ERROR) << "Failed to set async property " << info.name;

    if (PropertySet(info.name, info.value, &error) != PROP_SUCCESS) {

        LOG(ERROR) << "Failed to set async property " << info.name << " to " << info.value << ": "

                   << error;

    }

    }

    property_children.pop();

    property_children.pop();

    if (!property_children.empty()) {

    if (!property_children.empty()) {
@@ -216,9 +214,9 @@ bool PropertyChildReap(pid_t pid) { 
}

}





static uint32_t PropertySetAsync(const std::string& name, const std::string& value,

static uint32_t PropertySetAsync(const std::string& name, const std::string& value,

                                 PropertyAsyncFunc func) {

                                 PropertyAsyncFunc func, std::string* error) {

    if (value.empty()) {

    if (value.empty()) {

        return PropertySetImpl(name, value);

        return PropertySet(name, value, error);

    }

    }





    PropertyChildInfo info;

    PropertyChildInfo info;
@@ -236,30 +234,27 @@ static int RestoreconRecursiveAsync(const std::string& name, const std::string& 
    return selinux_android_restorecon(value.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE);

    return selinux_android_restorecon(value.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE);

}

}





uint32_t PropertySet(const std::string& name, const std::string& value) {

    if (name == "selinux.restorecon_recursive") {

        return PropertySetAsync(name, value, RestoreconRecursiveAsync);

    }



    return PropertySetImpl(name, value);

}



uint32_t InitPropertySet(const std::string& name, const std::string& value) {

uint32_t InitPropertySet(const std::string& name, const std::string& value) {

    if (StartsWith(name, "ctl.")) {

    if (StartsWith(name, "ctl.")) {

        LOG(ERROR) << "Do not set ctl. properties from init; call the Service functions directly";

        LOG(ERROR) << "InitPropertySet: Do not set ctl. properties from init; call the Service "

                      "functions directly";

        return PROP_ERROR_INVALID_NAME;

    }

    if (name == "selinux.restorecon_recursive") {

        LOG(ERROR) << "InitPropertySet: Do not set selinux.restorecon_recursive from init; use the "

                      "restorecon builtin directly";

        return PROP_ERROR_INVALID_NAME;

        return PROP_ERROR_INVALID_NAME;

    }

    }





    const char* type = nullptr;

    uint32_t result = 0;

    property_info_area->GetPropertyInfo(name.c_str(), nullptr, &type);

    ucred cr = {.pid = 1, .uid = 0, .gid = 0};



    std::string error;

    if (type == nullptr || !CheckType(type, value)) {

    result = HandlePropertySet(name, value, kInitContext.c_str(), cr, &error);

        LOG(ERROR) << "property_set: name: '" << name << "' type check failed, type: '"

    if (result != PROP_SUCCESS) {

                   << (type ?: "(null)") << "' value: '" << value << "'";

        LOG(ERROR) << "Init cannot set '" << name << "' to '" << value << "': " << error;

        return PROP_ERROR_INVALID_VALUE;

    }

    }





    return PropertySet(name, value);

    return result;

}

}





class SocketConnection {

class SocketConnection {
@@ -390,9 +385,9 @@ class SocketConnection { 




// This returns one of the enum of PROP_SUCCESS or PROP_ERROR*.

// This returns one of the enum of PROP_SUCCESS or PROP_ERROR*.

uint32_t HandlePropertySet(const std::string& name, const std::string& value,

uint32_t HandlePropertySet(const std::string& name, const std::string& value,

                           const std::string& source_context, const ucred& cr) {

                           const std::string& source_context, const ucred& cr, std::string* error) {

    if (!IsLegalPropertyName(name)) {

    if (!IsLegalPropertyName(name)) {

        LOG(ERROR) << "PropertySet: illegal property name \"" << name << "\"";

        *error = "Illegal property name";

        return PROP_ERROR_INVALID_NAME;

        return PROP_ERROR_INVALID_NAME;

    }

    }
@@ -405,9 +400,7 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, 
        const char* type = nullptr;

        const char* type = nullptr;

        property_info_area->GetPropertyInfo(control_string.c_str(), &target_context, &type);

        property_info_area->GetPropertyInfo(control_string.c_str(), &target_context, &type);

        if (!CheckMacPerms(control_string, target_context, source_context.c_str(), cr)) {

        if (!CheckMacPerms(control_string, target_context, source_context.c_str(), cr)) {

            LOG(ERROR) << "PropertySet: Unable to " << (name.c_str() + 4) << " service ctl ["

            *error = StringPrintf("Unable to '%s' service %s", name.c_str() + 4, value.c_str());

                       << value << "]"

                       << " uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid;

            return PROP_ERROR_HANDLE_CONTROL_MESSAGE;

            return PROP_ERROR_HANDLE_CONTROL_MESSAGE;

        }

        }
@@ -420,13 +413,13 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, 
    property_info_area->GetPropertyInfo(name.c_str(), &target_context, &type);

    property_info_area->GetPropertyInfo(name.c_str(), &target_context, &type);





    if (!CheckMacPerms(name, target_context, source_context.c_str(), cr)) {

    if (!CheckMacPerms(name, target_context, source_context.c_str(), cr)) {

        LOG(ERROR) << "PropertySet: permission denied uid:" << cr.uid << " name:" << name;

        *error = "SELinux permission check failed";

        return PROP_ERROR_PERMISSION_DENIED;

        return PROP_ERROR_PERMISSION_DENIED;

    }

    }





    if (type == nullptr || !CheckType(type, value)) {

    if (type == nullptr || !CheckType(type, value)) {

        LOG(ERROR) << "PropertySet: name: '" << name << "' type check failed, type: '"

        *error = StringPrintf("Property type check failed, value doesn't match expected type '%s'",

                   << (type ?: "(null)") << "' value: '" << value << "'";

                              (type ?: "(null)"));

        return PROP_ERROR_INVALID_VALUE;

        return PROP_ERROR_INVALID_VALUE;

    }

    }
@@ -445,7 +438,11 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, 
                  << process_log_string;

                  << process_log_string;

    }

    }





    return PropertySet(name, value);

    if (name == "selinux.restorecon_recursive") {

        return PropertySetAsync(name, value, RestoreconRecursiveAsync, error);

    }



    return PropertySet(name, value, error);

}

}





static void handle_property_set_fd() {

static void handle_property_set_fd() {
@@ -488,7 +485,16 @@ static void handle_property_set_fd() { 
        prop_name[PROP_NAME_MAX-1] = 0;

        prop_name[PROP_NAME_MAX-1] = 0;

        prop_value[PROP_VALUE_MAX-1] = 0;

        prop_value[PROP_VALUE_MAX-1] = 0;





        HandlePropertySet(prop_value, prop_value, socket.source_context(), socket.cred());

        const auto& cr = socket.cred();

        std::string error;

        uint32_t result =

            HandlePropertySet(prop_name, prop_value, socket.source_context(), cr, &error);

        if (result != PROP_SUCCESS) {

            LOG(ERROR) << "Unable to set property '" << prop_name << "' to '" << prop_value

                       << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": "

                       << error;

        }



        break;

        break;

      }

      }
@@ -502,7 +508,14 @@ static void handle_property_set_fd() { 
          return;

          return;

        }

        }





        auto result = HandlePropertySet(name, value, socket.source_context(), socket.cred());

        const auto& cr = socket.cred();

        std::string error;

        uint32_t result = HandlePropertySet(name, value, socket.source_context(), cr, &error);

        if (result != PROP_SUCCESS) {

            LOG(ERROR) << "Unable to set property '" << name << "' to '" << value

                       << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": "

                       << error;

        }

        socket.SendUint32(result);

        socket.SendUint32(result);

        break;

        break;

      }

      }

init/property_service.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -32,7 +32,7 @@ struct property_audit_data { 
extern uint32_t (*property_set)(const std::string& name, const std::string& value);

extern uint32_t (*property_set)(const std::string& name, const std::string& value);





uint32_t HandlePropertySet(const std::string& name, const std::string& value,

uint32_t HandlePropertySet(const std::string& name, const std::string& value,

                           const std::string& source_context, const ucred& cr);

                           const std::string& source_context, const ucred& cr, std::string* error);





extern bool PropertyChildReap(pid_t pid);

extern bool PropertyChildReap(pid_t pid);

init/subcontext.cpp

+5 −1
Original line number Original line Diff line number Diff line
@@ -297,7 +297,11 @@ Result<Success> Subcontext::Execute(const std::vector<std::string>& args) { 




    for (const auto& property : subcontext_reply->properties_to_set()) {

    for (const auto& property : subcontext_reply->properties_to_set()) {

        ucred cr = {.pid = pid_, .uid = 0, .gid = 0};

        ucred cr = {.pid = pid_, .uid = 0, .gid = 0};

        HandlePropertySet(property.name(), property.value(), context_, cr);

        std::string error;

        if (HandlePropertySet(property.name(), property.value(), context_, cr, &error) != 0) {

            LOG(ERROR) << "Subcontext init could not set '" << property.name() << "' to '"

                       << property.value() << "': " << error;

        }

    }

    }





    if (subcontext_reply->reply_case() == SubcontextReply::kFailure) {

    if (subcontext_reply->reply_case() == SubcontextReply::kFailure) {