* [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
@ 2022-02-18 11:05 Sergey Shtylyov
2022-02-18 11:17 ` Sergey Shtylyov
2022-02-19 2:39 ` Damien Le Moal
0 siblings, 2 replies; 6+ messages in thread
From: Sergey Shtylyov @ 2022-02-18 11:05 UTC (permalink / raw)
To: Damien Le Moal, linux-ide; +Cc: Dan Carpenter
Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
register doesn't exists, ata_port_warn() would print the 'status' variable
which never gets assigned. Restore the original order of the statements,
wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
While at it, fix crazy indentation in the ata_port_warn() call itself...
Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
drivers/ata/libata-sff.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
Index: libata/drivers/ata/libata-sff.c
===================================================================
--- libata.orig/drivers/ata/libata-sff.c
+++ libata/drivers/ata/libata-sff.c
@@ -1644,13 +1644,14 @@ void ata_sff_lost_interrupt(struct ata_p
return;
/* See if the controller thinks it is still busy - if so the command
isn't a lost IRQ but is still in progress */
- if (ata_sff_altstatus(ap, &status) && (status & ATA_BUSY))
+ if (WARN_ON_ONCE(!ata_sff_altstatus(ap, &status)))
+ return;
+ if (status & ATA_BUSY)
return;
/* There was a command running, we are no longer busy and we have
no interrupt. */
- ata_port_warn(ap, "lost interrupt (Status 0x%x)\n",
- status);
+ ata_port_warn(ap, "lost interrupt (Status 0x%x)\n", status);
/* Run the host interrupt logic as if the interrupt had not been
lost */
ata_sff_port_intr(ap, qc);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
2022-02-18 11:05 [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt() Sergey Shtylyov
@ 2022-02-18 11:17 ` Sergey Shtylyov
2022-02-18 23:00 ` Damien Le Moal
2022-02-19 2:39 ` Damien Le Moal
1 sibling, 1 reply; 6+ messages in thread
From: Sergey Shtylyov @ 2022-02-18 11:17 UTC (permalink / raw)
To: Damien Le Moal, linux-ide; +Cc: Dan Carpenter
On 2/18/22 2:05 PM, Sergey Shtylyov wrote:
> Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
> ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
> register doesn't exists, ata_port_warn() would print the 'status' variable
> which never gets assigned. Restore the original order of the statements,
> wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
>
> While at it, fix crazy indentation in the ata_port_warn() call itself...
>
> Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>
> ---
Forgot to mention that it's against the 'for-next' branch of 'libata.git'. :-/
[...]
MBR, Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
2022-02-18 11:17 ` Sergey Shtylyov
@ 2022-02-18 23:00 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-02-18 23:00 UTC (permalink / raw)
To: Sergey Shtylyov, linux-ide; +Cc: Dan Carpenter
On 2/18/22 20:17, Sergey Shtylyov wrote:
> On 2/18/22 2:05 PM, Sergey Shtylyov wrote:
>
>> Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
>> ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
>> register doesn't exists, ata_port_warn() would print the 'status' variable
>> which never gets assigned. Restore the original order of the statements,
>> wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
>>
>> While at it, fix crazy indentation in the ata_port_warn() call itself...
>>
>> Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> ---
>
> Forgot to mention that it's against the 'for-next' branch of 'libata.git'. :-/
No need to mention it, it always should be :)
>
> [...]
>
> MBR, Sergey
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
2022-02-18 11:05 [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt() Sergey Shtylyov
2022-02-18 11:17 ` Sergey Shtylyov
@ 2022-02-19 2:39 ` Damien Le Moal
2022-02-19 9:52 ` Sergey Shtylyov
1 sibling, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2022-02-19 2:39 UTC (permalink / raw)
To: Sergey Shtylyov, linux-ide; +Cc: Dan Carpenter
On 2/18/22 20:05, Sergey Shtylyov wrote:
> Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
> ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
> register doesn't exists, ata_port_warn() would print the 'status' variable
> which never gets assigned. Restore the original order of the statements,
> wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
>
> While at it, fix crazy indentation in the ata_port_warn() call itself...
>
> Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
I squashed this in the commit being fixed.
Dan,
Thanks for the report !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
2022-02-19 2:39 ` Damien Le Moal
@ 2022-02-19 9:52 ` Sergey Shtylyov
2022-02-19 23:01 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Shtylyov @ 2022-02-19 9:52 UTC (permalink / raw)
To: Damien Le Moal, linux-ide; +Cc: Dan Carpenter
On 2/19/22 5:39 AM, Damien Le Moal wrote:
>> Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
>> ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
>> register doesn't exists, ata_port_warn() would print the 'status' variable
>> which never gets assigned. Restore the original order of the statements,
>> wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
>>
>> While at it, fix crazy indentation in the ata_port_warn() call itself...
>>
>> Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>
> I squashed this in the commit being fixed.
I'm seeing a few typos/errors in the updated patch #2:
> In ata_sff_lost_interrupt(), wrap the ata_sff_altstatus() call in a
s/a/the/?
> WARN_ON_ONCE()
+ check?
> to issue a warning if the if the device control registert
Register? :-)
> does not exist. And while at it, fix crazy indentation in the
> ata_port_warn() call itself...
Not clear why you (we?) emphasize this by using "itself"...
Well, if you choose to fix those, I'll be thnakful. But you may as well
ignore me. :-)
[...]
MBR, Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt()
2022-02-19 9:52 ` Sergey Shtylyov
@ 2022-02-19 23:01 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-02-19 23:01 UTC (permalink / raw)
To: Sergey Shtylyov, linux-ide; +Cc: Dan Carpenter
On 2/19/22 18:52, Sergey Shtylyov wrote:
> On 2/19/22 5:39 AM, Damien Le Moal wrote:
>
>>> Due to my sloppy coding in commit 2c75a451ecb0 ("ata: libata-sff: refactor
>>> ata_sff_altstatus()"), in ata_sff_lost_interrupt() iff the device control
>>> register doesn't exists, ata_port_warn() would print the 'status' variable
>>> which never gets assigned. Restore the original order of the statements,
>>> wrapping the ata_sff_altstatus() call in WARN_ON_ONCE()...
>>>
>>> While at it, fix crazy indentation in the ata_port_warn() call itself...
>>>
>>> Fixes: 2c75a451ecb0 ("ata: libata-sff: refactor ata_sff_altstatus()")
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> I squashed this in the commit being fixed.
>
> I'm seeing a few typos/errors in the updated patch #2:
>
>> In ata_sff_lost_interrupt(), wrap the ata_sff_altstatus() call in a
>
> s/a/the/?
>
>> WARN_ON_ONCE()
>
> + check?
>
>> to issue a warning if the if the device control registert
>
> Register? :-)
>
>> does not exist. And while at it, fix crazy indentation in the
>> ata_port_warn() call itself...
>
> Not clear why you (we?) emphasize this by using "itself"...
>
> Well, if you choose to fix those, I'll be thnakful. But you may as well
> ignore me. :-)
Will fix that. Thanks for checking.
>
> [...]
>
> MBR, Sergey
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-19 23:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-18 11:05 [PATCH] ata: libata-sff: fix reading uninitialized variable in ata_sff_lost_interrupt() Sergey Shtylyov
2022-02-18 11:17 ` Sergey Shtylyov
2022-02-18 23:00 ` Damien Le Moal
2022-02-19 2:39 ` Damien Le Moal
2022-02-19 9:52 ` Sergey Shtylyov
2022-02-19 23:01 ` Damien Le Moal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox