* [PATCH 1/4] badblocks: goto out if find any unacked badblocks in _badblocks_check()
2023-12-23 6:37 [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() linan666
@ 2023-12-23 6:37 ` linan666
2023-12-23 6:37 ` [PATCH 2/4] badblocks: optimize _badblocks_check() linan666
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: linan666 @ 2023-12-23 6:37 UTC (permalink / raw)
To: axboe, geliang.tang, xni, colyli
Cc: ira.weiny, linux-block, linux-kernel, linan666, yukuai3, yi.zhang,
houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
In _badblocks_check(), after finding any unacked badblocks, the return
value, 'first_bad' and 'bad_sectors' will not be changed anymore, and
there is no need to check other ranges. So goto out directly after
finding unacked badblocks.
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index fc92d4e18aa3..ebf17a54851a 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -1274,7 +1274,7 @@ static int _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
sector_t *first_bad, int *bad_sectors)
{
- int unacked_badblocks, acked_badblocks;
+ int acked_badblocks;
int prev = -1, hint = -1, set = 0;
struct badblocks_context bad;
unsigned int seq;
@@ -1297,7 +1297,6 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
seq = read_seqbegin(&bb->lock);
p = bb->page;
- unacked_badblocks = 0;
acked_badblocks = 0;
re_check:
@@ -1318,21 +1317,24 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
}
if (overlap_front(bb, prev, &bad)) {
- if (BB_ACK(p[prev]))
+ if (set == 0) {
+ *first_bad = BB_OFFSET(p[prev]);
+ *bad_sectors = BB_LEN(p[prev]);
+ set = 1;
+ }
+
+ if (BB_ACK(p[prev])) {
acked_badblocks++;
- else
- unacked_badblocks++;
+ } else {
+ rv = -1;
+ goto out;
+ }
if (BB_END(p[prev]) >= (s + sectors))
len = sectors;
else
len = BB_END(p[prev]) - s;
- if (set == 0) {
- *first_bad = BB_OFFSET(p[prev]);
- *bad_sectors = BB_LEN(p[prev]);
- set = 1;
- }
goto update_sectors;
}
@@ -1355,13 +1357,12 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
WARN_ON(sectors < 0);
- if (unacked_badblocks > 0)
- rv = -1;
- else if (acked_badblocks > 0)
+ if (acked_badblocks > 0)
rv = 1;
else
rv = 0;
+out:
if (read_seqretry(&bb->lock, seq))
goto retry;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] badblocks: optimize _badblocks_check()
2023-12-23 6:37 [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() linan666
2023-12-23 6:37 ` [PATCH 1/4] badblocks: goto out if find any unacked badblocks in _badblocks_check() linan666
@ 2023-12-23 6:37 ` linan666
2023-12-23 6:37 ` [PATCH 3/4] badblocks: fix slab-out-of-bounds in _badblocks_check() linan666
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: linan666 @ 2023-12-23 6:37 UTC (permalink / raw)
To: axboe, geliang.tang, xni, colyli
Cc: ira.weiny, linux-block, linux-kernel, linan666, yukuai3, yi.zhang,
houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
Check badblocks_empty earlier, and goto out if check range starts after
all badblocks or badblocks is empty since no badblocks intersect with
check range.
Clean up redundant check '(prev + 1) < bb->count'. If it is true, it
will enter the earlier branch.
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index ebf17a54851a..054d05b93641 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -1296,6 +1296,11 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
retry:
seq = read_seqbegin(&bb->lock);
+ if (badblocks_empty(bb)) {
+ len = sectors;
+ goto out;
+ }
+
p = bb->page;
acked_badblocks = 0;
@@ -1303,17 +1308,12 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
bad.start = s;
bad.len = sectors;
- if (badblocks_empty(bb)) {
- len = sectors;
- goto update_sectors;
- }
-
prev = prev_badblocks(bb, &bad, hint);
/* start after all badblocks */
if ((prev + 1) >= bb->count && !overlap_front(bb, prev, &bad)) {
len = sectors;
- goto update_sectors;
+ goto out;
}
if (overlap_front(bb, prev, &bad)) {
@@ -1339,7 +1339,7 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
}
/* Not front overlap, but behind overlap */
- if ((prev + 1) < bb->count && overlap_behind(bb, &bad, prev + 1)) {
+ if (overlap_behind(bb, &bad, prev + 1)) {
len = BB_OFFSET(p[prev + 1]) - bad.start;
hint = prev + 1;
goto update_sectors;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] badblocks: fix slab-out-of-bounds in _badblocks_check()
2023-12-23 6:37 [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() linan666
2023-12-23 6:37 ` [PATCH 1/4] badblocks: goto out if find any unacked badblocks in _badblocks_check() linan666
2023-12-23 6:37 ` [PATCH 2/4] badblocks: optimize _badblocks_check() linan666
@ 2023-12-23 6:37 ` linan666
2023-12-23 6:37 ` [PATCH 4/4] badblocks: clean up prev_badblocks() linan666
2023-12-23 17:28 ` [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() Ira Weiny
4 siblings, 0 replies; 6+ messages in thread
From: linan666 @ 2023-12-23 6:37 UTC (permalink / raw)
To: axboe, geliang.tang, xni, colyli
Cc: ira.weiny, linux-block, linux-kernel, linan666, yukuai3, yi.zhang,
houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
prev_badblocks() will return -1 if checked range start < p[0]. Accessing
p[-1] will cause bug as below:
BUG: KASAN: slab-out-of-bounds in badblocks_check+0x2c4
Fix it by checking 'prev' before accessing badblocks->page.
Fixes: 3ea3354cb9f0 ("badblocks: improve badblocks_check() for multiple ranges handling")
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index 054d05b93641..71a3e43351da 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -1316,7 +1316,7 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
goto out;
}
- if (overlap_front(bb, prev, &bad)) {
+ if (prev >= 0 && overlap_front(bb, prev, &bad)) {
if (set == 0) {
*first_bad = BB_OFFSET(p[prev]);
*bad_sectors = BB_LEN(p[prev]);
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] badblocks: clean up prev_badblocks()
2023-12-23 6:37 [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() linan666
` (2 preceding siblings ...)
2023-12-23 6:37 ` [PATCH 3/4] badblocks: fix slab-out-of-bounds in _badblocks_check() linan666
@ 2023-12-23 6:37 ` linan666
2023-12-23 17:28 ` [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() Ira Weiny
4 siblings, 0 replies; 6+ messages in thread
From: linan666 @ 2023-12-23 6:37 UTC (permalink / raw)
To: axboe, geliang.tang, xni, colyli
Cc: ira.weiny, linux-block, linux-kernel, linan666, yukuai3, yi.zhang,
houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
The offset of 'lo' must <= ‘s' after biset. clean up redundant check.
And replace the check of badblocks->count with badblocks_empty().
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index 71a3e43351da..88ed13897443 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -486,7 +486,7 @@ static int prev_badblocks(struct badblocks *bb, struct badblocks_context *bad,
int lo, hi;
u64 *p;
- if (!bb->count)
+ if (badblocks_empty(bb))
goto out;
if (hint >= 0) {
@@ -521,8 +521,7 @@ static int prev_badblocks(struct badblocks *bb, struct badblocks_context *bad,
hi = mid;
}
- if (BB_OFFSET(p[lo]) <= s)
- ret = lo;
+ ret = lo;
out:
return ret;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check()
2023-12-23 6:37 [PATCH 0/4] badblocks: bugfix and cleanup of _badblocks_check() linan666
` (3 preceding siblings ...)
2023-12-23 6:37 ` [PATCH 4/4] badblocks: clean up prev_badblocks() linan666
@ 2023-12-23 17:28 ` Ira Weiny
4 siblings, 0 replies; 6+ messages in thread
From: Ira Weiny @ 2023-12-23 17:28 UTC (permalink / raw)
To: linan666, axboe, geliang.tang, xni, colyli
Cc: ira.weiny, linux-block, linux-kernel, linan666, yukuai3, yi.zhang,
houtao1, yangerkun
linan666@ wrote:
> From: Li Nan <linan122@huawei.com>
>
> Li Nan (4):
> badblocks: goto out if find any unacked badblocks in
> _badblocks_check()
> badblocks: optimize _badblocks_check()
> badblocks: fix slab-out-of-bounds in _badblocks_check()
> badblocks: clean up prev_badblocks()
>
> block/badblocks.c | 48 +++++++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> --
> 2.39.2
>
Thanks for the series! Unfortunately I'm still seeing some failures with
this series.
Coly's test patch[1] fixed all my test failures. Right off the top I'm
not seeing what you missed that he seemed to catch.
Ira
[1] https://lore.kernel.org/all/nhza4xsnbmcmka7463jxgmdvb27pqvbvcuzs7xp4vzpqlo262d@dp7laevqtaka/
^ permalink raw reply [flat|nested] 6+ messages in thread