[PATCH v2 0/6] UFS Advanced RPMB

[PATCH v2 0/6] UFS Advanced RPMB

[Bean Huo]

Changelog:

V1 --> V2:
    1. Added RPMB request completion handling in patch 6/6

RFC --> V1:
    1. Split the patch and Remove RFC
    2. Add all 8 types of rpmb operations
    3. Fix one EHS copy error in ufshcd_advanced_rpmb_req_handler()
    4. Fix several issues raised by Avri in the RFC patch:
    https://patchwork.kernel.org/project/linux-scsi/patch/20221107131038.201724-3-beanhuo@iokpp.de/#25081912

Bean Huo (6):
  ufs: ufs_bsg: Remove unnecessary length checkup
  ufs: ufs_bsg: Cleanup ufs_bsg_request
  ufs: core: Split ufshcd_map_sg
  ufs: core: Advanced RPMB detection
  ufs: core: Pass EHS length into ufshcd_prepare_req_desc_hdr()
  ufs: core: Add advanced RPMB support in ufs_bsg

 drivers/ufs/core/ufs_bsg.c       | 137 +++++++++++++++---------
 drivers/ufs/core/ufshcd.c        | 176 +++++++++++++++++++++++++------
 include/uapi/scsi/scsi_bsg_ufs.h |  46 +++++++-
 include/ufs/ufs.h                |  29 +++++
 include/ufs/ufshcd.h             |   6 +-
 include/ufs/ufshci.h             |   1 +
 6 files changed, 315 insertions(+), 80 deletions(-)

-- 
2.25.1

[PATCH v2 1/6] ufs: ufs_bsg: Remove unnecessary length checkup

[Bean Huo]

Remove checks on job->request_len and job->reply_len because
The following msgcode checks will rule out malicious requests.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufs_bsg.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index b99e3f3dc4ef..9ac8204f1ee6 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -30,21 +30,6 @@ static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
 	return 0;
 }
 
-static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
-				     unsigned int request_len,
-				     unsigned int reply_len)
-{
-	int min_req_len = sizeof(struct ufs_bsg_request);
-	int min_rsp_len = sizeof(struct ufs_bsg_reply);
-
-	if (min_req_len > request_len || min_rsp_len > reply_len) {
-		dev_err(hba->dev, "not enough space assigned\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 				     uint8_t **desc_buff, int *desc_len,
 				     enum query_opcode desc_op)
@@ -88,8 +73,6 @@ static int ufs_bsg_request(struct bsg_job *job)
 	struct ufs_bsg_request *bsg_request = job->request;
 	struct ufs_bsg_reply *bsg_reply = job->reply;
 	struct ufs_hba *hba = shost_priv(dev_to_shost(job->dev->parent));
-	unsigned int req_len = job->request_len;
-	unsigned int reply_len = job->reply_len;
 	struct uic_command uc = {};
 	int msgcode;
 	uint8_t *desc_buff = NULL;
@@ -97,10 +80,6 @@ static int ufs_bsg_request(struct bsg_job *job)
 	enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
 	int ret;
 
-	ret = ufs_bsg_verify_query_size(hba, req_len, reply_len);
-	if (ret)
-		goto out;
-
 	bsg_reply->reply_payload_rcv_len = 0;
 
 	ufshcd_rpm_get_sync(hba);
-- 
2.25.1

[PATCH v2 2/6] ufs: ufs_bsg: Cleanup ufs_bsg_request

[Bean Huo]

Move sg_copy_from_buffer() below its associated case statement.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufs_bsg.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 9ac8204f1ee6..850a0d798f63 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -90,11 +90,8 @@ static int ufs_bsg_request(struct bsg_job *job)
 		desc_op = bsg_request->upiu_req.qr.opcode;
 		ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
 						&desc_len, desc_op);
-		if (ret) {
-			ufshcd_rpm_put_sync(hba);
+		if (ret)
 			goto out;
-		}
-
 		fallthrough;
 	case UPIU_TRANSACTION_NOP_OUT:
 	case UPIU_TRANSACTION_TASK_REQ:
@@ -102,9 +99,12 @@ static int ufs_bsg_request(struct bsg_job *job)
 					       &bsg_reply->upiu_rsp, msgcode,
 					       desc_buff, &desc_len, desc_op);
 		if (ret)
-			dev_err(hba->dev,
-				"exe raw upiu: error code %d\n", ret);
-
+			dev_err(hba->dev, "exe raw upiu: error code %d\n", ret);
+		else if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len)
+			bsg_reply->reply_payload_rcv_len =
+				sg_copy_from_buffer(job->request_payload.sg_list,
+						    job->request_payload.sg_cnt,
+						    desc_buff, desc_len);
 		break;
 	case UPIU_TRANSACTION_UIC_CMD:
 		memcpy(&uc, &bsg_request->upiu_req.uc, UIC_CMD_SIZE);
@@ -123,20 +123,9 @@ static int ufs_bsg_request(struct bsg_job *job)
 		break;
 	}
 
+out:
 	ufshcd_rpm_put_sync(hba);
-
-	if (!desc_buff)
-		goto out;
-
-	if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len)
-		bsg_reply->reply_payload_rcv_len =
-			sg_copy_from_buffer(job->request_payload.sg_list,
-					    job->request_payload.sg_cnt,
-					    desc_buff, desc_len);
-
 	kfree(desc_buff);
-
-out:
 	bsg_reply->result = ret;
 	job->reply_len = sizeof(struct ufs_bsg_reply);
 	/* complete the job here only if no error */
-- 
2.25.1

RE: [PATCH v2 1/6] ufs: ufs_bsg: Remove unnecessary length checkup

[Avri Altman]

> 
> From: Bean Huo <beanhuo@micron.com>
> 
> Remove checks on job->request_len and job->reply_len because The following
> msgcode checks will rule out malicious requests.
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>
Acked-by: Avri Altman <avri.altman@wdc.com>


> ---
>  drivers/ufs/core/ufs_bsg.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c index
> b99e3f3dc4ef..9ac8204f1ee6 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -30,21 +30,6 @@ static int ufs_bsg_get_query_desc_size(struct ufs_hba
> *hba, int *desc_len,
>         return 0;
>  }
> 
> -static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
> -                                    unsigned int request_len,
> -                                    unsigned int reply_len)
> -{
> -       int min_req_len = sizeof(struct ufs_bsg_request);
> -       int min_rsp_len = sizeof(struct ufs_bsg_reply);
> -
> -       if (min_req_len > request_len || min_rsp_len > reply_len) {
> -               dev_err(hba->dev, "not enough space assigned\n");
> -               return -EINVAL;
> -       }
> -
> -       return 0;
> -}
> -
>  static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
>                                      uint8_t **desc_buff, int *desc_len,
>                                      enum query_opcode desc_op) @@ -88,8 +73,6 @@
> static int ufs_bsg_request(struct bsg_job *job)
>         struct ufs_bsg_request *bsg_request = job->request;
>         struct ufs_bsg_reply *bsg_reply = job->reply;
>         struct ufs_hba *hba = shost_priv(dev_to_shost(job->dev->parent));
> -       unsigned int req_len = job->request_len;
> -       unsigned int reply_len = job->reply_len;
>         struct uic_command uc = {};
>         int msgcode;
>         uint8_t *desc_buff = NULL;
> @@ -97,10 +80,6 @@ static int ufs_bsg_request(struct bsg_job *job)
>         enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
>         int ret;
> 
> -       ret = ufs_bsg_verify_query_size(hba, req_len, reply_len);
> -       if (ret)
> -               goto out;
> -
>         bsg_reply->reply_payload_rcv_len = 0;
> 
>         ufshcd_rpm_get_sync(hba);
> --
> 2.25.1

[PATCH v2 1/6] ufs: ufs_bsg: Remove unnecessary length checkup

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Remove checks on job->request_len and job->reply_len because
The following msgcode checks will rule out malicious requests.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufs_bsg.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index b99e3f3dc4ef..9ac8204f1ee6 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -30,21 +30,6 @@ static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
 	return 0;
 }
 
-static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
-				     unsigned int request_len,
-				     unsigned int reply_len)
-{
-	int min_req_len = sizeof(struct ufs_bsg_request);
-	int min_rsp_len = sizeof(struct ufs_bsg_reply);
-
-	if (min_req_len > request_len || min_rsp_len > reply_len) {
-		dev_err(hba->dev, "not enough space assigned\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 				     uint8_t **desc_buff, int *desc_len,
 				     enum query_opcode desc_op)
@@ -88,8 +73,6 @@ static int ufs_bsg_request(struct bsg_job *job)
 	struct ufs_bsg_request *bsg_request = job->request;
 	struct ufs_bsg_reply *bsg_reply = job->reply;
 	struct ufs_hba *hba = shost_priv(dev_to_shost(job->dev->parent));
-	unsigned int req_len = job->request_len;
-	unsigned int reply_len = job->reply_len;
 	struct uic_command uc = {};
 	int msgcode;
 	uint8_t *desc_buff = NULL;
@@ -97,10 +80,6 @@ static int ufs_bsg_request(struct bsg_job *job)
 	enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
 	int ret;
 
-	ret = ufs_bsg_verify_query_size(hba, req_len, reply_len);
-	if (ret)
-		goto out;
-
 	bsg_reply->reply_payload_rcv_len = 0;
 
 	ufshcd_rpm_get_sync(hba);
-- 
2.25.1