[PATCH REPOST net v2] atm/fore200e: Fix possible data race in fore200e_open()

[PATCH REPOST net v2] atm/fore200e: Fix possible data race in fore200e_open()

[Gui-Dong Han]

Protect access to fore200e->available_cell_rate with rate_mtx lock to
prevent potential data race.

In this case, since the update depends on a prior read, a data race
could lead to a wrong fore200e.available_cell_rate value.

The field fore200e.available_cell_rate is generally protected by the lock
fore200e.rate_mtx when accessed. In all other read and write cases, this
field is consistently protected by the lock, except for this case and
during initialization.

This potential bug was detected by our experimental static analysis tool,
which analyzes locking APIs and paired functions to identify data races
and atomicity violations.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
v2:
* Added a description of the data race hazard in fore200e_open(), as
suggested by Jakub Kicinski and Simon Horman.

REPOST:
* Reposting v2 as it seems to have been overlooked.
---
 drivers/atm/fore200e.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 4fea1149e003..f62e38571440 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -1374,7 +1374,9 @@ fore200e_open(struct atm_vcc *vcc)
 
 	vcc->dev_data = NULL;
 
+	mutex_lock(&fore200e->rate_mtx);
 	fore200e->available_cell_rate += vcc->qos.txtp.max_pcr;
+	mutex_unlock(&fore200e->rate_mtx);
 
 	kfree(fore200e_vcc);
 	return -EINVAL;
-- 
2.34.1

Re: [PATCH REPOST net v2] atm/fore200e: Fix possible data race in fore200e_open()

[Paolo Abeni]

On 11/18/25 4:33 AM, Gui-Dong Han wrote:
> Protect access to fore200e->available_cell_rate with rate_mtx lock to
> prevent potential data race.
> 
> In this case, since the update depends on a prior read, a data race
> could lead to a wrong fore200e.available_cell_rate value.
> 
> The field fore200e.available_cell_rate is generally protected by the lock
> fore200e.rate_mtx when accessed. In all other read and write cases, this
> field is consistently protected by the lock, except for this case and
> during initialization.
> 
> This potential bug was detected by our experimental static analysis tool,
> which analyzes locking APIs and paired functions to identify data races
> and atomicity violations.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> ---
> v2:
> * Added a description of the data race hazard in fore200e_open(), as
> suggested by Jakub Kicinski and Simon Horman.

It looks like you missed Jakub's reply on v2:

https://lore.kernel.org/netdev/20250123071201.3d38d8f6@kernel.org/

The above comment is still not sufficient: you should describe
accurately how 2 (or more) CPUs could actually race causing the
corruption, reporting the relevant call paths leading to the race.

Thanks,

Paolo

Re: [PATCH REPOST net v2] atm/fore200e: Fix possible data race in fore200e_open()

[Gui-Dong Han]

On Thu, Nov 20, 2025 at 7:26 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/18/25 4:33 AM, Gui-Dong Han wrote:
> > Protect access to fore200e->available_cell_rate with rate_mtx lock to
> > prevent potential data race.
> >
> > In this case, since the update depends on a prior read, a data race
> > could lead to a wrong fore200e.available_cell_rate value.
> >
> > The field fore200e.available_cell_rate is generally protected by the lock
> > fore200e.rate_mtx when accessed. In all other read and write cases, this
> > field is consistently protected by the lock, except for this case and
> > during initialization.
> >
> > This potential bug was detected by our experimental static analysis tool,
> > which analyzes locking APIs and paired functions to identify data races
> > and atomicity violations.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> > Reviewed-by: Simon Horman <horms@kernel.org>
> > ---
> > v2:
> > * Added a description of the data race hazard in fore200e_open(), as
> > suggested by Jakub Kicinski and Simon Horman.
>
> It looks like you missed Jakub's reply on v2:
>
> https://lore.kernel.org/netdev/20250123071201.3d38d8f6@kernel.org/
>
> The above comment is still not sufficient: you should describe
> accurately how 2 (or more) CPUs could actually race causing the
> corruption, reporting the relevant call paths leading to the race.

Hi Paolo,

Added the detailed description in v3.

Thank you,
Gui-Dong Han