[OpenWrt-Devel] [PATCH] netifd: More fine grained handling of unwanted proto shell actions

[Hans Dedecker]

In commit e1ec2d2b9e7f7692a4ff88a0361bbcdbe34f0d99 (proto-shell: extend race condition avoidance), changes were made to prevent notifications from the proto handler during the proto shell states teardown and abort.
These changes unfortunately also prevent netifd from being notified of a number of proto handler events like notify_error, blok_restart and kill during the teardown state.
This introduces issues when the protocol is ppp, the authfail UCI parameter is set and authentication fails; the interface autostart parameter cannot be set to disabled while the ppp daemon keeps running as these proto handler notifications are dropped in the teardown state.
The attached patch blocks the notifications in the individual notification handlers which should not get notifications during teardown/abort state and allows notify_error/blok_restart/kill notifications during these states.

Signed-off-by: Hans Dedecker <dedeckeh at gmail.com>
---
 proto-shell.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/proto-shell.c b/proto-shell.c
index 7a3e148..86dc9ce 100644
--- a/proto-shell.c
+++ b/proto-shell.c
@@ -440,6 +440,9 @@ proto_shell_update_link(struct proto_shell_state *state, struct blob_attr *data,
 	bool keep = false;
 	bool up;
 
+	if (state->sm == S_TEARDOWN || state->sm == S_SETUP_ABORT)
+		return UBUS_STATUS_PERMISSION_DENIED;
+
 	if (!tb[NOTIFY_LINK_UP])
 		return UBUS_STATUS_INVALID_ARGUMENT;
 
@@ -543,6 +546,9 @@ proto_shell_run_command(struct proto_shell_state *state, struct blob_attr **tb)
 	static char *argv[64];
 	static char *env[32];
 
+	if (state->sm == S_TEARDOWN || state->sm == S_SETUP_ABORT)
+		return UBUS_STATUS_PERMISSION_DENIED;
+
 	if (!tb[NOTIFY_COMMAND])
 		goto error;
 
@@ -641,6 +647,9 @@ proto_shell_add_host_dependency(struct proto_shell_state *state, struct blob_att
 	const char *ifname_str = ifname_a ? blobmsg_data(ifname_a) : "";
 	char *ifname;
 
+	if (state->sm == S_TEARDOWN || state->sm == S_SETUP_ABORT)
+		return UBUS_STATUS_PERMISSION_DENIED;
+
 	if (!host)
 		return UBUS_STATUS_INVALID_ARGUMENT;
 
@@ -670,6 +679,8 @@ proto_shell_add_host_dependency(struct proto_shell_state *state, struct blob_att
 static int
 proto_shell_setup_failed(struct proto_shell_state *state)
 {
+	int ret = 0;
+
 	switch (state->sm) {
 	case S_IDLE:
 		state->proto.proto_event(&state->proto, IFPEV_LINK_LOST);
@@ -677,10 +688,13 @@ proto_shell_setup_failed(struct proto_shell_state *state)
 	case S_SETUP:
 		proto_shell_handler(&state->proto, PROTO_CMD_TEARDOWN, false);
 		break;
+	case S_SETUP_ABORT:
+	case S_TEARDOWN:
 	default:
+		ret = UBUS_STATUS_PERMISSION_DENIED;
 		break;
 	}
-	return 0;
+	return ret;
 }
 
 static int
@@ -688,7 +702,6 @@ proto_shell_notify(struct interface_proto_state *proto, struct blob_attr *attr)
 {
 	struct proto_shell_state *state;
 	struct blob_attr *tb[__NOTIFY_LAST];
-	uint32_t action;
 
 	state = container_of(proto, struct proto_shell_state, proto);
 
@@ -696,13 +709,7 @@ proto_shell_notify(struct interface_proto_state *proto, struct blob_attr *attr)
 	if (!tb[NOTIFY_ACTION])
 		return UBUS_STATUS_INVALID_ARGUMENT;
 
-	action = blobmsg_get_u32(tb[NOTIFY_ACTION]);
-
-	/* allow proto_shell_notify_error even in S_TEARDOWN or S_SETUP_ABORT states */
-	if (action != 3 && (state->sm == S_TEARDOWN || state->sm == S_SETUP_ABORT))
-		return UBUS_STATUS_PERMISSION_DENIED;
-
-	switch(action) {
+	switch(blobmsg_get_u32(tb[NOTIFY_ACTION])) {
 	case 0:
 		return proto_shell_update_link(state, attr, tb);
 	case 1:
-- 
1.7.9.5
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel