Merge "Clean up property set error handling" (fe6cc42d) · Commits · e / os / android_system_core

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) {