* A possible divide by zero bug in blk_mq_map_queues
@ 2021-05-14 8:28 Yiyuan guo
2021-05-14 8:34 ` Hannes Reinecke
0 siblings, 1 reply; 5+ messages in thread
From: Yiyuan guo @ 2021-05-14 8:28 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Yiyuan guo
In block/blk-mq-cpumap.c, blk_mq_map_queues has the following code:
int blk_mq_map_queues(struct blk_mq_queue_map *qmap) {
...
unsigned int nr_queues = qmap->nr_queues;
unsigned q = 0;
...
for_each_present_cpu(cpu) {
if (q >= nr_queues)
break;
...
}
for_each_possible_cpu(cpu) {
...
if (q < nr_queues) {
map[cpu] = queue_index(qmap, nr_queues, q++);
} else {
...
if (first_sibling == cpu)
map[cpu] = queue_index(qmap, nr_queues, q++);
}
}
}
if qmap->nr_queues equals to zero when entering the function, then by
passing zero to function queue_index we have a divide by zero bug:
static int queue_index(struct blk_mq_queue_map *qmap,
unsigned int nr_queues, const int q)
{
return qmap->queue_offset + (q % nr_queues);
}
It seems possible to me that qmap->nr_queues may equal zero because
this field is explicitly checked in other functions.
For example, in the function blk_mq_map_swqueue (block/blk-mq.c),
there is a check comparing nr_queues with 0:
for (j = 0; j < set->nr_maps; j++) {
if (!set->map[j].nr_queues) {
...
continue;
}
...
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: A possible divide by zero bug in blk_mq_map_queues
2021-05-14 8:28 A possible divide by zero bug in blk_mq_map_queues Yiyuan guo
@ 2021-05-14 8:34 ` Hannes Reinecke
2021-05-14 9:16 ` [PATCH] block: add protection for divide by zero " Yiyuan GUO
0 siblings, 1 reply; 5+ messages in thread
From: Hannes Reinecke @ 2021-05-14 8:34 UTC (permalink / raw)
To: Yiyuan guo, axboe; +Cc: linux-block
On 5/14/21 10:28 AM, Yiyuan guo wrote:
> In block/blk-mq-cpumap.c, blk_mq_map_queues has the following code:
>
> int blk_mq_map_queues(struct blk_mq_queue_map *qmap) {
> ...
> unsigned int nr_queues = qmap->nr_queues;
> unsigned q = 0;
> ...
> for_each_present_cpu(cpu) {
> if (q >= nr_queues)
> break;
> ...
> }
>
> for_each_possible_cpu(cpu) {
> ...
> if (q < nr_queues) {
> map[cpu] = queue_index(qmap, nr_queues, q++);
> } else {
> ...
> if (first_sibling == cpu)
> map[cpu] = queue_index(qmap, nr_queues, q++);
>
> }
> }
> }
>
> if qmap->nr_queues equals to zero when entering the function, then by
> passing zero to function queue_index we have a divide by zero bug:
>
> static int queue_index(struct blk_mq_queue_map *qmap,
> unsigned int nr_queues, const int q)
> {
> return qmap->queue_offset + (q % nr_queues);
> }
>
> It seems possible to me that qmap->nr_queues may equal zero because
> this field is explicitly checked in other functions.
>
> For example, in the function blk_mq_map_swqueue (block/blk-mq.c),
> there is a check comparing nr_queues with 0:
>
> for (j = 0; j < set->nr_maps; j++) {
> if (!set->map[j].nr_queues) {
> ...
> continue;
> }
> ...
> }
>
Theoretically, but yes, possible.
Care to send a patch?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] block: add protection for divide by zero in blk_mq_map_queues
2021-05-14 8:34 ` Hannes Reinecke
@ 2021-05-14 9:16 ` Yiyuan GUO
2021-05-14 9:24 ` Hannes Reinecke
0 siblings, 1 reply; 5+ messages in thread
From: Yiyuan GUO @ 2021-05-14 9:16 UTC (permalink / raw)
To: hare; +Cc: axboe, linux-block, yguoaz, Yiyuan GUO
In function blk_mq_map_queues, qmap->nr_queues may equal zero
and thus it needs to be checked before we pass it to function
queue_index.
Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>
---
block/blk-mq-cpumap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 3db84d319..dc440870e 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -65,7 +65,8 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
} else {
first_sibling = get_first_sibling(cpu);
if (first_sibling == cpu)
- map[cpu] = queue_index(qmap, nr_queues, q++);
+ if (nr_queues)
+ map[cpu] = queue_index(qmap, nr_queues, q++);
else
map[cpu] = map[first_sibling];
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] block: add protection for divide by zero in blk_mq_map_queues
2021-05-14 9:16 ` [PATCH] block: add protection for divide by zero " Yiyuan GUO
@ 2021-05-14 9:24 ` Hannes Reinecke
2021-05-14 9:38 ` [PATCH v2] " Yiyuan GUO
0 siblings, 1 reply; 5+ messages in thread
From: Hannes Reinecke @ 2021-05-14 9:24 UTC (permalink / raw)
To: Yiyuan GUO; +Cc: axboe, linux-block, Yiyuan GUO
On 5/14/21 11:16 AM, Yiyuan GUO wrote:
> In function blk_mq_map_queues, qmap->nr_queues may equal zero
> and thus it needs to be checked before we pass it to function
> queue_index.
>
> Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>
> ---
> block/blk-mq-cpumap.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
> index 3db84d319..dc440870e 100644
> --- a/block/blk-mq-cpumap.c
> +++ b/block/blk-mq-cpumap.c
> @@ -65,7 +65,8 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
> } else {
> first_sibling = get_first_sibling(cpu);
> if (first_sibling == cpu)
> - map[cpu] = queue_index(qmap, nr_queues, q++);
> + if (nr_queues)
> + map[cpu] = queue_index(qmap, nr_queues, q++);
> else
> map[cpu] = map[first_sibling];
> }
>
Err ... and what is the value of 'map[cpu]' if 'nr_queues' equals zero?
Please move the 'if (nr_queues)' condition into the first if-clause:
if ((first_sibling == cpu) && nr_queues)
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] block: add protection for divide by zero in blk_mq_map_queues
2021-05-14 9:24 ` Hannes Reinecke
@ 2021-05-14 9:38 ` Yiyuan GUO
0 siblings, 0 replies; 5+ messages in thread
From: Yiyuan GUO @ 2021-05-14 9:38 UTC (permalink / raw)
To: hare; +Cc: axboe, linux-block, yguoaz, yguoaz
In function blk_mq_map_queues, qmap->nr_queues may equal zero
and thus it needs to be checked before we pass it to function
queue_index.
Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>
---
block/blk-mq-cpumap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 3db84d319..c2163658e 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -64,7 +64,7 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
map[cpu] = queue_index(qmap, nr_queues, q++);
} else {
first_sibling = get_first_sibling(cpu);
- if (first_sibling == cpu)
+ if ((first_sibling == cpu) && nr_queues)
map[cpu] = queue_index(qmap, nr_queues, q++);
else
map[cpu] = map[first_sibling];
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-14 9:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 8:28 A possible divide by zero bug in blk_mq_map_queues Yiyuan guo
2021-05-14 8:34 ` Hannes Reinecke
2021-05-14 9:16 ` [PATCH] block: add protection for divide by zero " Yiyuan GUO
2021-05-14 9:24 ` Hannes Reinecke
2021-05-14 9:38 ` [PATCH v2] " Yiyuan GUO
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.