[PATCH] block: bic maybe null pointer dereference

[PATCH] block: bic maybe null pointer dereference

[yanlonglong]

Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
---
 block/bfq-iosched.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 141c602d5e85..27ef736085b1 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -3035,9 +3035,6 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 {
 	struct bfq_io_cq *bic = bfqq->bic;
-	unsigned int a_idx = bfqq->actuator_idx;
-	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
-
 	/*
 	 * If !bfqq->bic, the queue is already shared or its requests
 	 * have already been redirected to a shared queue; both idle window
@@ -3046,6 +3043,9 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 	if (!bic)
 		return;
 
+	unsigned int a_idx = bfqq->actuator_idx;
+	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
+
 	bfqq_data->saved_last_serv_time_ns = bfqq->last_serv_time_ns;
 	bfqq_data->saved_inject_limit =	bfqq->inject_limit;
 	bfqq_data->saved_decrease_time_jif = bfqq->decrease_time_jif;
-- 
2.43.0

Re: [PATCH] block: bic maybe null pointer dereference

[Jens Axboe]

On 5/5/26 11:56 PM, yanlonglong wrote:
> Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>

Your subject line is incomplete, and your commit message is even more
incomplete.

> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 141c602d5e85..27ef736085b1 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -3035,9 +3035,6 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>  static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
>  {
>  	struct bfq_io_cq *bic = bfqq->bic;
> -	unsigned int a_idx = bfqq->actuator_idx;
> -	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
> -
>  	/*
>  	 * If !bfqq->bic, the queue is already shared or its requests
>  	 * have already been redirected to a shared queue; both idle window
> @@ -3046,6 +3043,9 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
>  	if (!bic)
>  		return;
>  
> +	unsigned int a_idx = bfqq->actuator_idx;
> +	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
> +
>  	bfqq_data->saved_last_serv_time_ns = bfqq->last_serv_time_ns;
>  	bfqq_data->saved_inject_limit =	bfqq->inject_limit;
>  	bfqq_data->saved_decrease_time_jif = bfqq->decrease_time_jif;

Ehm no, variable declarations go at the top.

Please try again, correcting the title/subject and ACTUALLY write a
commit message. See:

Documentation/process/submitting-patches.rst

-- 
Jens Axboe

[PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function

[yanlonglong]

When the `bic` variable is null, referencing `bfqq_data` through `bic` will
cause the program to crash. Therefore, the null check for `bic` should be
moved to the beginning of the function to prevent referencing a null pointer.

Fixed:fd571df0ac5b289af8（"block, bfq: turn bfqq_data into an array in bfq_io_cq"）
Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
---
 block/bfq-iosched.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 141c602d5e85..e952e4ea2dd4 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -3035,9 +3035,8 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 {
 	struct bfq_io_cq *bic = bfqq->bic;
-	unsigned int a_idx = bfqq->actuator_idx;
-	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
-
+	unsigned int a_idx = 0;
+	struct bfq_iocq_bfqq_data *bfqq_data = NULL;
 	/*
 	 * If !bfqq->bic, the queue is already shared or its requests
 	 * have already been redirected to a shared queue; both idle window
@@ -3046,6 +3045,9 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 	if (!bic)
 		return;
 
+	a_idx = bfqq->actuator_idx;
+	bfqq_data = &bic->bfqq_data[a_idx];
+
 	bfqq_data->saved_last_serv_time_ns = bfqq->last_serv_time_ns;
 	bfqq_data->saved_inject_limit =	bfqq->inject_limit;
 	bfqq_data->saved_decrease_time_jif = bfqq->decrease_time_jif;
-- 
2.43.0

Re: [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function

[Jens Axboe]

On 5/6/26 3:04 AM, yanlonglong wrote:
> When the `bic` variable is null, referencing `bfqq_data` through `bic` will
> cause the program to crash. Therefore, the null check for `bic` should be
> moved to the beginning of the function to prevent referencing a null pointer.

Cap at 72 char line length... And title should start with "block, bfq: ".
And should be "add NULL check", singular, see below.

> Fixed:fd571df0ac5b289af8?"block, bfq: turn bfqq_data into an array in bfq_io_cq"?

This tag is wrong, should be a shortened sha and the format is also
wrong (Fixed vs Fixes).

> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 141c602d5e85..e952e4ea2dd4 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -3035,9 +3035,8 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>  static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
>  {
>  	struct bfq_io_cq *bic = bfqq->bic;
> -	unsigned int a_idx = bfqq->actuator_idx;
> -	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
> -
> +	unsigned int a_idx = 0;
> +	struct bfq_iocq_bfqq_data *bfqq_data = NULL;

You're killing the empty line between variables and the comment. And why
is a_idx being moved? This is !bic being NULL, presumably?

-- 
Jens Axboe

[PATCH v3] block: add NULL checks for bic in bfq_bfqq_save_state function

[yanlonglong]

When the `bic` variable is null, referencing `bfqq_data` through `bic` will
cause the program to crash. Therefore, the null check for `bic` should be
moved to the beginning of the function to prevent referencing a null pointer.

Fixes:fd571df0ac5b("block, bfq: turn bfqq_data into an array in bfq_io_cq")
Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
---
 block/bfq-iosched.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 141c602d5e85..c8cf8764d48d 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -3036,7 +3036,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 {
 	struct bfq_io_cq *bic = bfqq->bic;
 	unsigned int a_idx = bfqq->actuator_idx;
-	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
+	struct bfq_iocq_bfqq_data *bfqq_data = NULL;
 
 	/*
 	 * If !bfqq->bic, the queue is already shared or its requests
@@ -3046,6 +3046,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 	if (!bic)
 		return;
 
+	bfqq_data = &bic->bfqq_data[a_idx];
 	bfqq_data->saved_last_serv_time_ns = bfqq->last_serv_time_ns;
 	bfqq_data->saved_inject_limit =	bfqq->inject_limit;
 	bfqq_data->saved_decrease_time_jif = bfqq->decrease_time_jif;
-- 
2.43.0