[PATCH AUTOSEL 4.14 1/3] drm/exynos: fix a possible null-pointer dereference due to data race in exynos_drm_crtc_atomic_disable()
From: Tuo Li islituo@gmail.com
[ Upstream commit 2e63972a2de14482d0eae1a03a73e379f1c3f44c ]
The variable crtc->state->event is often protected by the lock crtc->dev->event_lock when is accessed. However, it is accessed as a condition of an if statement in exynos_drm_crtc_atomic_disable() without holding the lock:
if (crtc->state->event && !crtc->state->active)
However, if crtc->state->event is changed to NULL by another thread right after the conditions of the if statement is checked to be true, a null-pointer dereference can occur in drm_crtc_send_vblank_event():
e->pipe = pipe;
To fix this possible null-pointer dereference caused by data race, the spin lock coverage is extended to protect the if statement as well as the function call to drm_crtc_send_vblank_event().
Reported-by: BassCheck bass@buaa.edu.cn Link: https://sites.google.com/view/basscheck/home Signed-off-by: Tuo Li islituo@gmail.com Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Added relevant link. Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 4787560bf93e7..e1aa518ea0ba1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -43,13 +43,12 @@ static void exynos_drm_crtc_atomic_disable(struct drm_crtc *crtc, if (exynos_crtc->ops->disable) exynos_crtc->ops->disable(exynos_crtc);
+ spin_lock_irq(&crtc->dev->event_lock); if (crtc->state->event && !crtc->state->active) { - spin_lock_irq(&crtc->dev->event_lock); drm_crtc_send_vblank_event(crtc, crtc->state->event); - spin_unlock_irq(&crtc->dev->event_lock); - crtc->state->event = NULL; } + spin_unlock_irq(&crtc->dev->event_lock); }
static int exynos_crtc_atomic_check(struct drm_crtc *crtc,
From: Zhang Shurong zhang_shurong@foxmail.com
[ Upstream commit 8b0472b50bcf0f19a5119b00a53b63579c8e1e4d ]
If rddev->raid_disk is greater than mddev->raid_disks, there will be an out-of-bounds in raid1_remove_disk(). We have already found similar reports as follows:
1) commit d17f744e883b ("md-raid10: fix KASAN warning") 2) commit 1ebc2cec0b7d ("dm raid: fix KASAN warning in raid5_remove_disk")
Fix this bug by checking whether the "number" variable is valid.
Signed-off-by: Zhang Shurong zhang_shurong@foxmail.com Reviewed-by: Yu Kuai yukuai3@huawei.com Link: https://lore.kernel.org/r/tencent_0D24426FAC6A21B69AC0C03CE4143A508F09@qq.co... Signed-off-by: Song Liu song@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/md/raid1.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index 28f78199de3ba..3e54b6639e213 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1775,6 +1775,10 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev) struct r1conf *conf = mddev->private; int err = 0; int number = rdev->raid_disk; + + if (unlikely(number >= conf->raid_disks)) + goto abort; + struct raid1_info *p = conf->mirrors + number;
if (rdev != p->rdev)
Hi!
From: Zhang Shurong zhang_shurong@foxmail.com
[ Upstream commit 8b0472b50bcf0f19a5119b00a53b63579c8e1e4d ]
If rddev->raid_disk is greater than mddev->raid_disks, there will be an out-of-bounds in raid1_remove_disk(). We have already found similar reports as follows:
- commit d17f744e883b ("md-raid10: fix KASAN warning")
- commit 1ebc2cec0b7d ("dm raid: fix KASAN warning in raid5_remove_disk")
Fix this bug by checking whether the "number" variable is valid.
+++ b/drivers/md/raid1.c @@ -1775,6 +1775,10 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev) struct r1conf *conf = mddev->private; int err = 0; int number = rdev->raid_disk;
- if (unlikely(number >= conf->raid_disks))
goto abort;- struct raid1_info *p = conf->mirrors + number;
if (rdev != p->rdev)
Wow. Mixing declarations and code. I'm pretty sure that's not ok according to our coding style, and I'd be actually surprised if all our compiler configurations allowed this.
Best regards, Pavel
On Mon, Sep 11, 2023 at 2:49 AM Pavel Machek pavel@denx.de wrote:
Hi!
From: Zhang Shurong zhang_shurong@foxmail.com
[ Upstream commit 8b0472b50bcf0f19a5119b00a53b63579c8e1e4d ]
If rddev->raid_disk is greater than mddev->raid_disks, there will be an out-of-bounds in raid1_remove_disk(). We have already found similar reports as follows:
- commit d17f744e883b ("md-raid10: fix KASAN warning")
- commit 1ebc2cec0b7d ("dm raid: fix KASAN warning in raid5_remove_disk")
Fix this bug by checking whether the "number" variable is valid.
+++ b/drivers/md/raid1.c @@ -1775,6 +1775,10 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev) struct r1conf *conf = mddev->private; int err = 0; int number = rdev->raid_disk;
if (unlikely(number >= conf->raid_disks))goto abort;struct raid1_info *p = conf->mirrors + number; if (rdev != p->rdev)Wow. Mixing declarations and code. I'm pretty sure that's not ok according to our coding style, and I'd be actually surprised if all our compiler configurations allowed this.
We have a fix queued for this:
https://git.kernel.org/pub/scm/linux/kernel/git/song/md.git/commit/?h=md-fix...
Thanks, Song
From: Georg Ottinger g.ottinger@gmx.at
[ Upstream commit e88076348425b7d0491c8c98d8732a7df8de7aa3 ]
I run a small server that uses external hard drives for backups. The backup software I use uses ext2 filesystems with 4KiB block size and the server is running SELinux and therefore relies on xattr. I recently upgraded the hard drives from 4TB to 12TB models. I noticed that after transferring some TBs I got a filesystem error "Freeing blocks not in datazone - block = 18446744071529317386, count = 1" and the backup process stopped. Trying to fix the fs with e2fsck resulted in a completely corrupted fs. The error probably came from ext2_free_blocks(), and because of the large number 18e19 this problem immediately looked like some kind of integer overflow. Whereas the 4TB fs was about 1e9 blocks, the new 12TB is about 3e9 blocks. So, searching the ext2 code, I came across the line in fs/ext2/xattr.c:745 where ext2_new_block() is called and the resulting block number is stored in the variable block as an int datatype. If a block with a block number greater than INT32_MAX is returned, this variable overflows and the call to sb_getblk() at line fs/ext2/xattr.c:750 fails, then the call to ext2_free_blocks() produces the error.
Signed-off-by: Georg Ottinger g.ottinger@gmx.at Signed-off-by: Jan Kara jack@suse.cz Message-Id: 20230815100340.22121-1-g.ottinger@gmx.at Signed-off-by: Sasha Levin sashal@kernel.org --- fs/ext2/xattr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c index bd1d68ff3a9f8..437175bce22e8 100644 --- a/fs/ext2/xattr.c +++ b/fs/ext2/xattr.c @@ -664,10 +664,10 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh, /* We need to allocate a new block */ ext2_fsblk_t goal = ext2_group_first_block_no(sb, EXT2_I(inode)->i_block_group); - int block = ext2_new_block(inode, goal, &error); + ext2_fsblk_t block = ext2_new_block(inode, goal, &error); if (error) goto cleanup; - ea_idebug(inode, "creating block %d", block); + ea_idebug(inode, "creating block %lu", block);
new_bh = sb_getblk(sb, block); if (unlikely(!new_bh)) {
linux-stable-mirror@lists.linaro.org
-
Pavel Machek -
Sasha Levin -
Song Liu