subject:"\[linux\-yocto\]\[linux\-yocto v5.15\/standard\/base\]\[PATCH 1\/1\] mtd_blkdevs\: add mtd_table_mutex lock back to blktrans_\{open, release\} to avoid race condition"

Re: [linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

2022-09-09 Thread Bruce Ashfield

In message: Re: [linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] 
mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid 
race condition
on 09/09/2022 Paul Gortmaker wrote:

> [[linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add 
> mtd_table_mutex lock back to blktrans_{open, release} to avoid race 
> condition] On 09/09/2022 (Fri 16:26) LiweiSong wrote:
> 
> > without lock mtd_table_mutex in blktrans_{open, release}, there will
> > be a race condition when access devices /dev/mtd1 and /dev/mtdblock1
> > at the same time with a high frequency open and close test:
> > 
> > kernel BUG at drivers/mtd/mtdcore.c:1221!
> > Internal error: Oops - BUG: 0 1 PREEMPT_RT SMP
> > CPU: 0 PID: 15349 Comm: mtd-test Not tainted 5.15.52-rt41-yocto-preempt-rt 
> > #1
> > Hardware name: SoCFPGA Stratix 10 SoCDK (DT)
> > pstate: 6005put_mtd_device+0x4c/0x84
> > lr : blktrans_release+0xb0/0x120
> > Call trace:
> > __put_mtd_device+0x4c/0x84
> > blktrans_release+0xb0/0x120
> > blkdev_put+0xd4/0x210
> > blkdev_close+0x34/0x50
> > __fput+0x8c/0x240
> > fput+0x1c/0x30
> > task_work_run+0x98/00t_64_sync_handler+0xa4/0x130
> > el0t_64_sync+0x1a0/0x1a4
> 
> In the interest of transparency - ongoing discussions continue on how
> best to fix this in mainline - albeit slowly.
> 
> https://lore.kernel.org/all/cf66e306-f216-5247-7cff-36ce08eb4...@windriver.com/
> 
> So even while this may not be the eventual mainline solution, the
> thought here is that it is a viable temporary one that resolves the
> failing test case(s) for Yocto and has no known negative side effects.

Thanks Paul! You saved me digging around for why this was coming
from a staging branch.

I agree with the analysis and have merged the change.

Bruce

> 
> Thanks,
> Paul.
> --
> 
> > 
> > since the original purpose of commit 799ae31c58ae ("mtd_blkdevs:
> > don't hold del_mtd_blktrans_dev in blktrans_{open, release}") is
> > to fix a DEADLOCK issue, here convert mutex_lock to mutex_trylock
> > and return immediately if failed acquire mtd_table_mutex, then
> > both race condition and DEADLOCK can be avoided.
> > 
> > Fixes: 799ae31c58ae ("mtd_blkdevs: don't hold del_mtd_blktrans_dev in 
> > blktrans_{open, release}")
> > Signed-off-by: Liwei Song 
> > ---
> >  drivers/mtd/mtd_blkdevs.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> > index b8ae1ec14e17..147e4a11dfe4 100644
> > --- a/drivers/mtd/mtd_blkdevs.c
> > +++ b/drivers/mtd/mtd_blkdevs.c
> > @@ -188,6 +188,8 @@ static int blktrans_open(struct block_device *bdev, 
> > fmode_t mode)
> >  
> > kref_get(>ref);
> >  
> > +   if (!mutex_trylock(_table_mutex))
> > +   return ret;
> > mutex_lock(>lock);
> >  
> > if (dev->open)
> > @@ -212,6 +214,7 @@ static int blktrans_open(struct block_device *bdev, 
> > fmode_t mode)
> >  unlock:
> > dev->open++;
> > mutex_unlock(>lock);
> > +   mutex_unlock(_table_mutex);
> > return ret;
> >  
> >  error_release:
> > @@ -220,6 +223,7 @@ static int blktrans_open(struct block_device *bdev, 
> > fmode_t mode)
> >  error_put:
> > module_put(dev->tr->owner);
> > mutex_unlock(>lock);
> > +   mutex_unlock(_table_mutex);
> > blktrans_dev_put(dev);
> > return ret;
> >  }
> > @@ -228,6 +232,8 @@ static void blktrans_release(struct gendisk *disk, 
> > fmode_t mode)
> >  {
> > struct mtd_blktrans_dev *dev = disk->private_data;
> >  
> > +   if (!mutex_trylock(_table_mutex))
> > +   return;
> > mutex_lock(>lock);
> >  
> > if (--dev->open)
> > @@ -242,6 +248,7 @@ static void blktrans_release(struct gendisk *disk, 
> > fmode_t mode)
> > }
> >  unlock:
> > mutex_unlock(>lock);
> > +   mutex_unlock(_table_mutex);
> > blktrans_dev_put(dev);
> >  }
> >  
> > -- 
> > 2.36.1
> > 
> 
> > 
> > 
> > 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#11648): 
https://lists.yoctoproject.org/g/linux-yocto/message/11648
Mute This Topic: https://lists.yoctoproject.org/mt/93568504/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

2022-09-09 Thread Paul Gortmaker

[[linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add 
mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition] 
On 09/09/2022 (Fri 16:26) LiweiSong wrote:

> without lock mtd_table_mutex in blktrans_{open, release}, there will
> be a race condition when access devices /dev/mtd1 and /dev/mtdblock1
> at the same time with a high frequency open and close test:
> 
> kernel BUG at drivers/mtd/mtdcore.c:1221!
> Internal error: Oops - BUG: 0 1 PREEMPT_RT SMP
> CPU: 0 PID: 15349 Comm: mtd-test Not tainted 5.15.52-rt41-yocto-preempt-rt #1
> Hardware name: SoCFPGA Stratix 10 SoCDK (DT)
> pstate: 6005put_mtd_device+0x4c/0x84
> lr : blktrans_release+0xb0/0x120
> Call trace:
> __put_mtd_device+0x4c/0x84
> blktrans_release+0xb0/0x120
> blkdev_put+0xd4/0x210
> blkdev_close+0x34/0x50
> __fput+0x8c/0x240
> fput+0x1c/0x30
> task_work_run+0x98/00t_64_sync_handler+0xa4/0x130
> el0t_64_sync+0x1a0/0x1a4

In the interest of transparency - ongoing discussions continue on how
best to fix this in mainline - albeit slowly.

https://lore.kernel.org/all/cf66e306-f216-5247-7cff-36ce08eb4...@windriver.com/

So even while this may not be the eventual mainline solution, the
thought here is that it is a viable temporary one that resolves the
failing test case(s) for Yocto and has no known negative side effects.

Thanks,
Paul.
--

> 
> since the original purpose of commit 799ae31c58ae ("mtd_blkdevs:
> don't hold del_mtd_blktrans_dev in blktrans_{open, release}") is
> to fix a DEADLOCK issue, here convert mutex_lock to mutex_trylock
> and return immediately if failed acquire mtd_table_mutex, then
> both race condition and DEADLOCK can be avoided.
> 
> Fixes: 799ae31c58ae ("mtd_blkdevs: don't hold del_mtd_blktrans_dev in 
> blktrans_{open, release}")
> Signed-off-by: Liwei Song 
> ---
>  drivers/mtd/mtd_blkdevs.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index b8ae1ec14e17..147e4a11dfe4 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -188,6 +188,8 @@ static int blktrans_open(struct block_device *bdev, 
> fmode_t mode)
>  
>   kref_get(>ref);
>  
> + if (!mutex_trylock(_table_mutex))
> + return ret;
>   mutex_lock(>lock);
>  
>   if (dev->open)
> @@ -212,6 +214,7 @@ static int blktrans_open(struct block_device *bdev, 
> fmode_t mode)
>  unlock:
>   dev->open++;
>   mutex_unlock(>lock);
> + mutex_unlock(_table_mutex);
>   return ret;
>  
>  error_release:
> @@ -220,6 +223,7 @@ static int blktrans_open(struct block_device *bdev, 
> fmode_t mode)
>  error_put:
>   module_put(dev->tr->owner);
>   mutex_unlock(>lock);
> + mutex_unlock(_table_mutex);
>   blktrans_dev_put(dev);
>   return ret;
>  }
> @@ -228,6 +232,8 @@ static void blktrans_release(struct gendisk *disk, 
> fmode_t mode)
>  {
>   struct mtd_blktrans_dev *dev = disk->private_data;
>  
> + if (!mutex_trylock(_table_mutex))
> + return;
>   mutex_lock(>lock);
>  
>   if (--dev->open)
> @@ -242,6 +248,7 @@ static void blktrans_release(struct gendisk *disk, 
> fmode_t mode)
>   }
>  unlock:
>   mutex_unlock(>lock);
> + mutex_unlock(_table_mutex);
>   blktrans_dev_put(dev);
>  }
>  
> -- 
> 2.36.1
> 

> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#11642): 
https://lists.yoctoproject.org/g/linux-yocto/message/11642
Mute This Topic: https://lists.yoctoproject.org/mt/93568504/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

2022-09-09 Thread LiweiSong

without lock mtd_table_mutex in blktrans_{open, release}, there will
be a race condition when access devices /dev/mtd1 and /dev/mtdblock1
at the same time with a high frequency open and close test:

kernel BUG at drivers/mtd/mtdcore.c:1221!
Internal error: Oops - BUG: 0 1 PREEMPT_RT SMP
CPU: 0 PID: 15349 Comm: mtd-test Not tainted 5.15.52-rt41-yocto-preempt-rt #1
Hardware name: SoCFPGA Stratix 10 SoCDK (DT)
pstate: 6005put_mtd_device+0x4c/0x84
lr : blktrans_release+0xb0/0x120
Call trace:
__put_mtd_device+0x4c/0x84
blktrans_release+0xb0/0x120
blkdev_put+0xd4/0x210
blkdev_close+0x34/0x50
__fput+0x8c/0x240
fput+0x1c/0x30
task_work_run+0x98/00t_64_sync_handler+0xa4/0x130
el0t_64_sync+0x1a0/0x1a4

since the original purpose of commit 799ae31c58ae ("mtd_blkdevs:
don't hold del_mtd_blktrans_dev in blktrans_{open, release}") is
to fix a DEADLOCK issue, here convert mutex_lock to mutex_trylock
and return immediately if failed acquire mtd_table_mutex, then
both race condition and DEADLOCK can be avoided.

Fixes: 799ae31c58ae ("mtd_blkdevs: don't hold del_mtd_blktrans_dev in 
blktrans_{open, release}")
Signed-off-by: Liwei Song 
---
 drivers/mtd/mtd_blkdevs.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index b8ae1ec14e17..147e4a11dfe4 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -188,6 +188,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t 
mode)
 
kref_get(>ref);
 
+   if (!mutex_trylock(_table_mutex))
+   return ret;
mutex_lock(>lock);
 
if (dev->open)
@@ -212,6 +214,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t 
mode)
 unlock:
dev->open++;
mutex_unlock(>lock);
+   mutex_unlock(_table_mutex);
return ret;
 
 error_release:
@@ -220,6 +223,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t 
mode)
 error_put:
module_put(dev->tr->owner);
mutex_unlock(>lock);
+   mutex_unlock(_table_mutex);
blktrans_dev_put(dev);
return ret;
 }
@@ -228,6 +232,8 @@ static void blktrans_release(struct gendisk *disk, fmode_t 
mode)
 {
struct mtd_blktrans_dev *dev = disk->private_data;
 
+   if (!mutex_trylock(_table_mutex))
+   return;
mutex_lock(>lock);
 
if (--dev->open)
@@ -242,6 +248,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t 
mode)
}
 unlock:
mutex_unlock(>lock);
+   mutex_unlock(_table_mutex);
blktrans_dev_put(dev);
 }
 
-- 
2.36.1


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#11641): 
https://lists.yoctoproject.org/g/linux-yocto/message/11641
Mute This Topic: https://lists.yoctoproject.org/mt/93568504/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

Re: [linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

[linux-yocto][linux-yocto v5.15/standard/base][PATCH 1/1] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

3 matches

Site Navigation

Mail list logo

Footer information