linux-kernel - Re: [PATCH] scsi: pm80xx: Fix race condition caused by static variables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANxonXNtbo5UoaT0cvdZxAHo2jnmoQn7bzaZ2dk7_=Pf=M6LPA@mail.gmail.com>
Date: Mon, 11 Aug 2025 14:22:30 -0700
From: Francisco Gutierrez Ramirez <frankramirez@...gle.com>
To: "Martin K. Petersen" <martin.petersen@...cle.com>
Cc: Jack Wang <jinpu.wang@...ud.ionos.com>, 
	"James E . J . Bottomley" <James.Bottomley@...senpartnership.com>, linux-scsi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jerry.Wong@...rochip.com, vishakhavc@...gle.com
Subject: Re: [PATCH] scsi: pm80xx: Fix race condition caused by static variables

Hi Martin,

Thanks very much for taking the time to review my patch and for your feedback.

I'm writing to provide a bit more context for the design. The function
in question needs to be called repeatedly, and the struct helps to
track state across those calls without using static variables, instead
tying the state to the specific controller instance. This ensures the
function is thread-safe.

Please let me know if that context clarifies my approach, or if you
have an alternative suggestion for managing state safely in this
scenario.

Thanks again for your help.

Best regards,
Francisco Gutierrez

On Tue, Aug 5, 2025 at 7:30 PM Martin K. Petersen
<martin.petersen@...cle.com> wrote:
>
>
> Francisco,
>
> > -     for (; start < end; start++)
> > -             str += sprintf(str, "%08x ", *(temp+start));
> > -     count++;
>
> > +     for (; pm8001_ha->iop_log_start < pm8001_ha->iop_log_end; pm8001_ha->iop_log_start++)
> > +             str += sprintf(str, "%08x ", *(temp+pm8001_ha->iop_log_start));
> > +     pm8001_ha->iop_log_count++;
>
> Since the start, end, and count variables are only used in this
> function, what is the point of putting them in 'struct pm8001_hba_info'?
> It makes the lines longer and harder to read...
>
> --
> Martin K. Petersen