From: Yu Kuai <yukuai3@huawei.com>

__xa_cmpxchg() is called with rcu_read_lock(), and it will allocated
memory if necessary.

Fix the problem by moving rcu_read_lock() after __xa_cmpxchg, meanwhile,
it still should be held before xa_unlock(), prevent returned page to be
freed by concurrent discard.

Fixes: bbcacab2e8ee ("brd: avoid extra xarray lookups on first write")
Reported-by: syzbot+ea4c8fd177a47338881a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/685ec4c9.a00a0220.129264.000c.GAE@google.com/
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/block/brd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index b1be6c510372..0c2eabe14af3 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -64,13 +64,15 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector,
 
 	rcu_read_unlock();
 	page = alloc_page(gfp | __GFP_ZERO | __GFP_HIGHMEM);
-	rcu_read_lock();
-	if (!page)
+	if (!page) {
+		rcu_read_lock();
 		return ERR_PTR(-ENOMEM);
+	}
 
 	xa_lock(&brd->brd_pages);
 	ret = __xa_cmpxchg(&brd->brd_pages, sector >> PAGE_SECTORS_SHIFT, NULL,
 			page, gfp);
+	rcu_read_lock();
 	if (ret) {
 		xa_unlock(&brd->brd_pages);
 		__free_page(page);
-- 
2.39.2

s/leeping/sleeping/ in the subject.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

From: Yu Kuai <yukuai3@huawei.com>

__xa_cmpxchg() is called with rcu_read_lock(), and it will allocated
memory if necessary.

Fix the problem by moving rcu_read_lock() after __xa_cmpxchg, meanwhile,
it still should be held before xa_unlock(), prevent returned page to be
freed by concurrent discard.

Fixes: bbcacab2e8ee ("brd: avoid extra xarray lookups on first write")
Reported-by: syzbot+ea4c8fd177a47338881a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/685ec4c9.a00a0220.129264.000c.GAE@google.com/
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/block/brd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index b1be6c510372..0c2eabe14af3 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -64,13 +64,15 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector,
 
 	rcu_read_unlock();
 	page = alloc_page(gfp | __GFP_ZERO | __GFP_HIGHMEM);
-	rcu_read_lock();
-	if (!page)
+	if (!page) {
+		rcu_read_lock();
 		return ERR_PTR(-ENOMEM);
+	}
 
 	xa_lock(&brd->brd_pages);
 	ret = __xa_cmpxchg(&brd->brd_pages, sector >> PAGE_SECTORS_SHIFT, NULL,
 			page, gfp);
+	rcu_read_lock();
 	if (ret) {
 		xa_unlock(&brd->brd_pages);
 		__free_page(page);
-- 
2.39.2

Sorry that I somehow send this patch twice. Please ignore the redundant
one.

Thanks,
Kuai

在 2025/06/28 9:14, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> __xa_cmpxchg() is called with rcu_read_lock(), and it will allocated
> memory if necessary.
> 
> Fix the problem by moving rcu_read_lock() after __xa_cmpxchg, meanwhile,
> it still should be held before xa_unlock(), prevent returned page to be
> freed by concurrent discard.
> 
> Fixes: bbcacab2e8ee ("brd: avoid extra xarray lookups on first write")
> Reported-by: syzbot+ea4c8fd177a47338881a@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/685ec4c9.a00a0220.129264.000c.GAE@google.com/
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   drivers/block/brd.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index b1be6c510372..0c2eabe14af3 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -64,13 +64,15 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector,
>   
>   	rcu_read_unlock();
>   	page = alloc_page(gfp | __GFP_ZERO | __GFP_HIGHMEM);
> -	rcu_read_lock();
> -	if (!page)
> +	if (!page) {
> +		rcu_read_lock();
>   		return ERR_PTR(-ENOMEM);
> +	}
>   
>   	xa_lock(&brd->brd_pages);
>   	ret = __xa_cmpxchg(&brd->brd_pages, sector >> PAGE_SECTORS_SHIFT, NULL,
>   			page, gfp);
> +	rcu_read_lock();
>   	if (ret) {
>   		xa_unlock(&brd->brd_pages);
>   		__free_page(page);
>