* [PATCH] block: bic maybe null pointer dereference
@ 2026-05-06 5:56 yanlonglong
2026-05-06 7:09 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: yanlonglong @ 2026-05-06 5:56 UTC (permalink / raw)
To: yukuai, axboe; +Cc: linux-block, linux-kernel, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] block: bic maybe null pointer dereference
2026-05-06 5:56 [PATCH] block: bic maybe null pointer dereference yanlonglong
@ 2026-05-06 7:09 ` Jens Axboe
2026-05-06 9:04 ` [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function yanlonglong
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2026-05-06 7:09 UTC (permalink / raw)
To: yanlonglong, yukuai; +Cc: linux-block, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function
2026-05-06 7:09 ` Jens Axboe
@ 2026-05-06 9:04 ` yanlonglong
2026-05-06 10:41 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: yanlonglong @ 2026-05-06 9:04 UTC (permalink / raw)
To: axboe; +Cc: linux-block, linux-kernel, yanlonglong, yukuai
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function
2026-05-06 9:04 ` [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function yanlonglong
@ 2026-05-06 10:41 ` Jens Axboe
2026-05-07 1:28 ` [PATCH v3] " yanlonglong
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2026-05-06 10:41 UTC (permalink / raw)
To: yanlonglong; +Cc: linux-block, linux-kernel, yukuai
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] block: add NULL checks for bic in bfq_bfqq_save_state function
2026-05-06 10:41 ` Jens Axboe
@ 2026-05-07 1:28 ` yanlonglong
2026-05-07 5:18 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: yanlonglong @ 2026-05-07 1:28 UTC (permalink / raw)
To: axboe; +Cc: linux-block, linux-kernel, yanlonglong, yukuai
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] block: add NULL checks for bic in bfq_bfqq_save_state function
2026-05-07 1:28 ` [PATCH v3] " yanlonglong
@ 2026-05-07 5:18 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2026-05-07 5:18 UTC (permalink / raw)
To: yanlonglong; +Cc: linux-block, linux-kernel, yukuai
On 5/6/26 7:28 PM, 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.
This part is obvious, what really needs explaining here is what conditions
can lead to bic being NULL, as that is supposedly the problem being fixed.
So, which conditions lead to bic being NULL here? A good commit message
should explain the "why" of why a change is being made. It's obvious
from the code change what is being done, what is not obvious is why it's
necessary.
> Fixes:fd571df0ac5b("block, bfq: turn bfqq_data into an array in bfq_io_cq")
Still incorrect, need space after Fixes:
> 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;
Minor nit, but why initialize it to NULL?
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-07 5:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 5:56 [PATCH] block: bic maybe null pointer dereference yanlonglong
2026-05-06 7:09 ` Jens Axboe
2026-05-06 9:04 ` [PATCH v2] block: add NULL checks for bic in bfq_bfqq_save_state function yanlonglong
2026-05-06 10:41 ` Jens Axboe
2026-05-07 1:28 ` [PATCH v3] " yanlonglong
2026-05-07 5:18 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox