lkml.org 
[lkml]   [2024]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] [media] xc4000: Fix atomicity violation in xc4000_get_frequency
From
Dear All:

I hope this email finds you well. I hope you haven't missed my previous
email, as I understand that everyone has a busy schedule. I just wanted
to follow up on my previous message sent.
I understand that you may be occupied with other tasks or priorities.
However, I would greatly appreciate it if you could spare a few moments
to check the patch in my previous email. Your prompt response would be
highly valuable to me.
Thank you for your attention to this matter, and I look forward to
hearing from you soon.

Thanks,
Han

On 22/12/2023 下午1:50, Gui-Dong Han wrote:
> In xc4000_get_frequency():
> *freq = priv->freq_hz + priv->freq_offset;
> The code accesses priv->freq_hz and priv->freq_offset without holding any
> lock.
>
> In xc4000_set_params():
> // Code that updates priv->freq_hz and priv->freq_offset
> ...
>
> xc4000_get_frequency() and xc4000_set_params() may execute concurrently,
> risking inconsistent reads of priv->freq_hz and priv->freq_offset. Since
> these related data may update during reading, it can result in incorrect
> frequency calculation, leading to atomicity violations.
>
> This possible bug is found by an experimental static analysis tool
> developed by our team, BassCheck[1]. This tool analyzes the locking APIs
> to extract function pairs that can be concurrently executed, and then
> analyzes the instructions in the paired functions to identify possible
> concurrency bugs including data races and atomicity violations. The above
> possible bug is reported when our tool analyzes the source code of
> Linux 6.2.
>
> To address this issue, it is proposed to add a mutex lock pair in
> xc4000_get_frequency() to ensure atomicity. With this patch applied, our
> tool no longer reports the possible bug, with the kernel configuration
> allyesconfig for x86_64. Due to the lack of associated hardware, we cannot
> test the patch in runtime testing, and just verify it according to the
> code logic.
>
> [1] https://sites.google.com/view/basscheck/
>
> Fixes: 4c07e32884ab6 ("[media] xc4000: Fix get_frequency()")
> Cc: stable@vger.kernel.org
> Reported-by: BassCheck <bass@buaa.edu.cn>
> Signed-off-by: Gui-Dong Han <2045gemini@gmail.com>
> ---
> v2:
> * In this patch v2, we've added some information of the static analysis
> tool used, as per the researcher guidelines. Also, we've added a cc in the
> signed-off-by area, according to the stable-kernel-rules.
> Thank Greg KH for helpful advice.
> ---
> drivers/media/tuners/xc4000.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/tuners/xc4000.c b/drivers/media/tuners/xc4000.c
> index 57ded9ff3f04..29bc63021c5a 100644
> --- a/drivers/media/tuners/xc4000.c
> +++ b/drivers/media/tuners/xc4000.c
> @@ -1515,10 +1515,10 @@ static int xc4000_get_frequency(struct dvb_frontend *fe, u32 *freq)
> {
> struct xc4000_priv *priv = fe->tuner_priv;
>
> + mutex_lock(&priv->lock);
> *freq = priv->freq_hz + priv->freq_offset;
>
> if (debug) {
> - mutex_lock(&priv->lock);
> if ((priv->cur_fw.type
> & (BASE | FM | DTV6 | DTV7 | DTV78 | DTV8)) == BASE) {
> u16 snr = 0;
> @@ -1529,8 +1529,8 @@ static int xc4000_get_frequency(struct dvb_frontend *fe, u32 *freq)
> return 0;
> }
> }
> - mutex_unlock(&priv->lock);
> }
> + mutex_unlock(&priv->lock);
>
> dprintk(1, "%s()\n", __func__);
>

\
 
 \ /
  Last update: 2024-01-11 02:40    [W:0.061 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site