[PATCH] hv: utils: handle and propagate errors in kvp_register

[PATCH] hv: utils: handle and propagate errors in kvp_register

[Thorsten Blum]

Make kvp_register() return an error code instead of silently ignoring
failures, and propagate the error from kvp_handle_handshake() instead of
returning success.

This propagates both kzalloc_obj() and hvutil_transport_send() failures
to kvp_handle_handshake() and thus to kvp_on_msg().

Fixes: 245ba56a52a3 ("Staging: hv: Implement key/value pair (KVP)")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/hv/hv_kvp.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 0d73daf745a7..6180ebe040ff 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -93,7 +93,7 @@ static void kvp_send_key(struct work_struct *dummy);
 static void kvp_respond_to_host(struct hv_kvp_msg *msg, int error);
 static void kvp_timeout_func(struct work_struct *dummy);
 static void kvp_host_handshake_func(struct work_struct *dummy);
-static void kvp_register(int);
+static int kvp_register(int);
 
 static DECLARE_DELAYED_WORK(kvp_timeout_work, kvp_timeout_func);
 static DECLARE_DELAYED_WORK(kvp_host_handshake_work, kvp_host_handshake_func);
@@ -127,24 +127,26 @@ static void kvp_register_done(void)
 	hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
 }
 
-static void
+static int
 kvp_register(int reg_value)
 {
 
 	struct hv_kvp_msg *kvp_msg;
 	char *version;
+	int ret;
 
 	kvp_msg = kzalloc_obj(*kvp_msg);
+	if (!kvp_msg)
+		return -ENOMEM;
 
-	if (kvp_msg) {
-		version = kvp_msg->body.kvp_register.version;
-		kvp_msg->kvp_hdr.operation = reg_value;
-		strcpy(version, HV_DRV_VERSION);
+	version = kvp_msg->body.kvp_register.version;
+	kvp_msg->kvp_hdr.operation = reg_value;
+	strcpy(version, HV_DRV_VERSION);
 
-		hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg),
-				      kvp_register_done);
-		kfree(kvp_msg);
-	}
+	ret = hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg),
+				    kvp_register_done);
+	kfree(kvp_msg);
+	return ret;
 }
 
 static void kvp_timeout_func(struct work_struct *dummy)
@@ -186,9 +188,8 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
 	 */
 	pr_debug("KVP: userspace daemon ver. %d connected\n",
 		 msg->kvp_hdr.operation);
-	kvp_register(dm_reg_value);
 
-	return 0;
+	return kvp_register(dm_reg_value);
 }
 
 

RE: [EXTERNAL] [PATCH] hv: utils: handle and propagate errors in kvp_register

[Long Li]

> Make kvp_register() return an error code instead of silently ignoring failures, and
> propagate the error from kvp_handle_handshake() instead of returning success.
> 
> This propagates both kzalloc_obj() and hvutil_transport_send() failures to
> kvp_handle_handshake() and thus to kvp_on_msg().
> 
> Fixes: 245ba56a52a3 ("Staging: hv: Implement key/value pair (KVP)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

Reviewed-by:  Long Li <longli@microsoft.com>


> ---
>  drivers/hv/hv_kvp.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> 0d73daf745a7..6180ebe040ff 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -93,7 +93,7 @@ static void kvp_send_key(struct work_struct *dummy);
> static void kvp_respond_to_host(struct hv_kvp_msg *msg, int error);  static void
> kvp_timeout_func(struct work_struct *dummy);  static void
> kvp_host_handshake_func(struct work_struct *dummy); -static void
> kvp_register(int);
> +static int kvp_register(int);
> 
>  static DECLARE_DELAYED_WORK(kvp_timeout_work, kvp_timeout_func);  static
> DECLARE_DELAYED_WORK(kvp_host_handshake_work,
> kvp_host_handshake_func); @@ -127,24 +127,26 @@ static void
> kvp_register_done(void)
>  	hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);  }
> 
> -static void
> +static int
>  kvp_register(int reg_value)
>  {
> 
>  	struct hv_kvp_msg *kvp_msg;
>  	char *version;
> +	int ret;
> 
>  	kvp_msg = kzalloc_obj(*kvp_msg);
> +	if (!kvp_msg)
> +		return -ENOMEM;
> 
> -	if (kvp_msg) {
> -		version = kvp_msg->body.kvp_register.version;
> -		kvp_msg->kvp_hdr.operation = reg_value;
> -		strcpy(version, HV_DRV_VERSION);
> +	version = kvp_msg->body.kvp_register.version;
> +	kvp_msg->kvp_hdr.operation = reg_value;
> +	strcpy(version, HV_DRV_VERSION);
> 
> -		hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg),
> -				      kvp_register_done);
> -		kfree(kvp_msg);
> -	}
> +	ret = hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg),
> +				    kvp_register_done);
> +	kfree(kvp_msg);
> +	return ret;
>  }
> 
>  static void kvp_timeout_func(struct work_struct *dummy) @@ -186,9 +188,8
> @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
>  	 */
>  	pr_debug("KVP: userspace daemon ver. %d connected\n",
>  		 msg->kvp_hdr.operation);
> -	kvp_register(dm_reg_value);
> 
> -	return 0;
> +	return kvp_register(dm_reg_value);
>  }
> 
>