Hello,

This is Jonghwi from Samsung. :)
I am sending you a patch via new email as requested.


bsg: initialize request and reply payloads in bsg_prepare_job

struct bsg_job payloads contain fields that are only populated by
certain commands, such as sg_list pointers.

Because struct bsg_job is allocated with kmalloc(), memory may be
reused across requests. If a command does not populate all payload
fields, stale state from a previous job may remain and later be
misinterpreted during cleanup, potentially leading to use-after-free
or double-free issues.

Initialize both request and reply payloads at the beginning of job
preparation to ensure a clean state for all commands.

Signed-off-by: Jonghwi Rha <jonghwi.rha@samsung.com>

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 32da4a4429ce..0fbf8e311c03 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req)
        struct bsg_job *job = blk_mq_rq_to_pdu(req);
        int ret;

+       /* Clear stale SG state since bsg_job is reused as a request PDU */
+       job->request_payload.sg_list = NULL;
+       job->request_payload.sg_cnt = 0;
+       job->reply_payload.sg_list = NULL;
+       job->reply_payload.sg_cnt = 0;
+
        job->timeout = req->timeout;

        if (req->bio) {


BRs,
Jonghwi,

On 2/4/26 10:32 PM, ??? wrote:
> bsg: initialize request and reply payloads in bsg_prepare_job
> 
> struct bsg_job payloads contain fields that are only populated by
> certain commands, such as sg_list pointers.
> 
> Because struct bsg_job is allocated with kmalloc(), memory may be
> reused across requests. If a command does not populate all payload
> fields, stale state from a previous job may remain and later be
> misinterpreted during cleanup, potentially leading to use-after-free
> or double-free issues.
> 
> Initialize both request and reply payloads at the beginning of job
> preparation to ensure a clean state for all commands.
> 
> Signed-off-by: Jonghwi Rha <jonghwi.rha@samsung.com>
> 
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index 32da4a4429ce..0fbf8e311c03 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req)
>         struct bsg_job *job = blk_mq_rq_to_pdu(req);
>         int ret;
> 
> +       /* Clear stale SG state since bsg_job is reused as a request PDU */
> +       job->request_payload.sg_list = NULL;
> +       job->request_payload.sg_cnt = 0;
> +       job->reply_payload.sg_list = NULL;
> +       job->reply_payload.sg_cnt = 0;
> +
>         job->timeout = req->timeout;
> 
>         if (req->bio) {

The patch is white-space damaged, tabs are spaces. But I can fix that
up. Do we just want to do a memset(job, 0, sizeof(*job)) here to avoid
any oddities like this in the future?

-- 
Jens Axboe

On 2/5/26 14:42, Jens Axboe wrote:
> On 2/4/26 10:32 PM, ??? wrote:
>> bsg: initialize request and reply payloads in bsg_prepare_job
>>
>> struct bsg_job payloads contain fields that are only populated by
>> certain commands, such as sg_list pointers.
>>
>> Because struct bsg_job is allocated with kmalloc(), memory may be
>> reused across requests. If a command does not populate all payload
>> fields, stale state from a previous job may remain and later be
>> misinterpreted during cleanup, potentially leading to use-after-free
>> or double-free issues.
>>
>> Initialize both request and reply payloads at the beginning of job
>> preparation to ensure a clean state for all commands.
>>
>> Signed-off-by: Jonghwi Rha <jonghwi.rha@samsung.com>
>>
>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
>> index 32da4a4429ce..0fbf8e311c03 100644
>> --- a/block/bsg-lib.c
>> +++ b/block/bsg-lib.c
>> @@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req)
>>          struct bsg_job *job = blk_mq_rq_to_pdu(req);
>>          int ret;
>>
>> +       /* Clear stale SG state since bsg_job is reused as a request PDU */
>> +       job->request_payload.sg_list = NULL;
>> +       job->request_payload.sg_cnt = 0;
>> +       job->reply_payload.sg_list = NULL;
>> +       job->reply_payload.sg_cnt = 0;
>> +
>>          job->timeout = req->timeout;
>>
>>          if (req->bio) {
> 
> The patch is white-space damaged, tabs are spaces. But I can fix that
> up. Do we just want to do a memset(job, 0, sizeof(*job)) here to avoid
> any oddities like this in the future?
> 

That might indeed be better.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

On 2/6/26 00:45, Hannes Reinecke wrote:
> On 2/5/26 14:42, Jens Axboe wrote:
>> On 2/4/26 10:32 PM, ??? wrote:
>>> bsg: initialize request and reply payloads in bsg_prepare_job
>>>
>>> struct bsg_job payloads contain fields that are only populated by
>>> certain commands, such as sg_list pointers.
>>>
>>> Because struct bsg_job is allocated with kmalloc(), memory may be
>>> reused across requests. If a command does not populate all payload
>>> fields, stale state from a previous job may remain and later be
>>> misinterpreted during cleanup, potentially leading to use-after-free
>>> or double-free issues.
>>>
>>> Initialize both request and reply payloads at the beginning of job
>>> preparation to ensure a clean state for all commands.
>>>
>>> Signed-off-by: Jonghwi Rha <jonghwi.rha@samsung.com>
>>>
>>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
>>> index 32da4a4429ce..0fbf8e311c03 100644
>>> --- a/block/bsg-lib.c
>>> +++ b/block/bsg-lib.c
>>> @@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req)
>>>          struct bsg_job *job = blk_mq_rq_to_pdu(req);
>>>          int ret;
>>>
>>> +      /* Clear stale SG state since bsg_job is reused as a request PDU */
>>> +      job->request_payload.sg_list = NULL;
>>> +      job->request_payload.sg_cnt = 0;
>>> +      job->reply_payload.sg_list = NULL;
>>> +      job->reply_payload.sg_cnt = 0;
>>> +
>>>          job->timeout = req->timeout;
>>>
>>>          if (req->bio) {
>>
>> The patch is white-space damaged, tabs are spaces. But I can fix that
>> up. Do we just want to do a memset(job, 0, sizeof(*job)) here to avoid
>> any oddities like this in the future?
>>
>
> That might indeed be better.

The suggested method impairs normal operation. If bsg_prepare_job performs a zero‑memset for the job structure, all request‑related information set on the driver side before the call will be lost. Therefore, if it runs as is, it will go to ufs_bsg_request and cause a null‑pointer access.

Currently, the original patch has no functional impact.

The blank problem seems to be due to a mistake I made while copying and pasting the patch. I am reattaching the patch below. If needed, I can attach the patch and resend the new email.


[PATCH] bsg: initialize request and reply payloads in bsg_prepare_job

struct bsg_job payloads contain fields that are only populated by
certain commands, such as sg_list pointers.

Because struct bsg_job is allocated with kmalloc(), memory may be
reused across requests. If a command does not populate all payload
fields, stale state from a previous job may remain and later be
misinterpreted during cleanup, potentially leading to use-after-free
or double-free issues.

Initialize both request and reply payloads at the beginning of job
preparation to ensure a clean state for all commands.

Signed-off-by: Jonghwi Rha <jonghwi.rha@samsung.com>
---
 block/bsg-lib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 32da4a4429ce..0fbf8e311c03 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req)
 	struct bsg_job *job = blk_mq_rq_to_pdu(req);
 	int ret;
 
+	/* Clear stale SG state since bsg_job is reused as a request PDU */
+	job->request_payload.sg_list = NULL;
+	job->request_payload.sg_cnt = 0;
+	job->reply_payload.sg_list = NULL;
+	job->reply_payload.sg_cnt = 0;
+
 	job->timeout = req->timeout;
 
 	if (req->bio) {
-- 

Regards,
Jonghwi,