libata: Fix a potential race condition in ata_scsi_park_show()

libata: Fix a potential race condition in ata_scsi_park_show()

[Elias Oltmanns]

Peter Moulder has pointed out that there is a slight chance that a
negative value might be passed to jiffies_to_msecs() in
ata_scsi_park_show(). This is fixed by saving the value of jiffies in a
local variable, thus also reducing code since the volatile variable
jiffies is accessed only once.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---
Applies to your upstream branch.

 drivers/ata/libata-scsi.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index fccd5e4..e1f916f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -190,7 +190,7 @@ static ssize_t ata_scsi_park_show(struct device *device,
 	struct ata_port *ap;
 	struct ata_link *link;
 	struct ata_device *dev;
-	unsigned long flags;
+	unsigned long flags, now;
 	unsigned int uninitialized_var(msecs);
 	int rc = 0;
 
@@ -208,10 +208,11 @@ static ssize_t ata_scsi_park_show(struct device *device,
 	}
 
 	link = dev->link;
+	now = jiffies;
 	if (ap->pflags & ATA_PFLAG_EH_IN_PROGRESS &&
 	    link->eh_context.unloaded_mask & (1 << dev->devno) &&
-	    time_after(dev->unpark_deadline, jiffies))
-		msecs = jiffies_to_msecs(dev->unpark_deadline - jiffies);
+	    time_after(dev->unpark_deadline, now))
+		msecs = jiffies_to_msecs(dev->unpark_deadline - now);
 	else
 		msecs = 0;
 

Re: libata: Fix a potential race condition in ata_scsi_park_show()

[Tejun Heo]

Elias Oltmanns wrote:
> Peter Moulder has pointed out that there is a slight chance that a
> negative value might be passed to jiffies_to_msecs() in
> ata_scsi_park_show(). This is fixed by saving the value of jiffies in a
> local variable, thus also reducing code since the volatile variable
> jiffies is accessed only once.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

added to #tj-upstream.  Thanks.

-- 
tejun

Re: libata: Fix a potential race condition in ata_scsi_park_show()

[Elias Oltmanns]

Tejun Heo <tj@kernel.org> wrote:
> Elias Oltmanns wrote:
>> Peter Moulder has pointed out that there is a slight chance that a
>
>> negative value might be passed to jiffies_to_msecs() in
>> ata_scsi_park_show(). This is fixed by saving the value of jiffies in a
>> local variable, thus also reducing code since the volatile variable
>> jiffies is accessed only once.
>> 
>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>
> added to #tj-upstream.  Thanks.

Has this branch been pulled in yet? My patch, for one, is not even in
linux-next even though it had been intended for 2.6.28 at the time.

Regards,

Elias

Re: libata: Fix a potential race condition in ata_scsi_park_show()

[Tejun Heo]

Elias Oltmanns wrote:
> Tejun Heo <tj@kernel.org> wrote:
>> Elias Oltmanns wrote:
>>> Peter Moulder has pointed out that there is a slight chance that a
>>> negative value might be passed to jiffies_to_msecs() in
>>> ata_scsi_park_show(). This is fixed by saving the value of jiffies in a
>>> local variable, thus also reducing code since the volatile variable
>>> jiffies is accessed only once.
>>>
>>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>> added to #tj-upstream.  Thanks.
> 
> Has this branch been pulled in yet? My patch, for one, is not even in
> linux-next even though it had been intended for 2.6.28 at the time.

Jeff, have you pulled it or taken patches from it?

Thanks.

-- 
tejun

Re: libata: Fix a potential race condition in ata_scsi_park_show()

[Jeff Garzik]

Tejun Heo wrote:
> Elias Oltmanns wrote:
>> Tejun Heo <tj@kernel.org> wrote:
>>> Elias Oltmanns wrote:
>>>> Peter Moulder has pointed out that there is a slight chance that a
>>>> negative value might be passed to jiffies_to_msecs() in
>>>> ata_scsi_park_show(). This is fixed by saving the value of jiffies in a
>>>> local variable, thus also reducing code since the volatile variable
>>>> jiffies is accessed only once.
>>>>
>>>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>>> added to #tj-upstream.  Thanks.
>> Has this branch been pulled in yet? My patch, for one, is not even in
>> linux-next even though it had been intended for 2.6.28 at the time.
> 
> Jeff, have you pulled it or taken patches from it?
> 
> Thanks.

I sent an email a couple days ago, asking what was pending (or thought I 
did...).  I haven't pulled from #tj-upstream but I ack'd a big patchset 
that I assume is included in there.

	Jeff

Re: libata: Fix a potential race condition in ata_scsi_park_show()

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Elias Oltmanns wrote:
>>> Tejun Heo <tj@kernel.org> wrote:
>>>> Elias Oltmanns wrote:
>>>>> Peter Moulder has pointed out that there is a slight chance that a
>>>>> negative value might be passed to jiffies_to_msecs() in
>>>>> ata_scsi_park_show(). This is fixed by saving the value of jiffies
>>>>> in a
>>>>> local variable, thus also reducing code since the volatile variable
>>>>> jiffies is accessed only once.
>>>>>
>>>>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>>>> added to #tj-upstream.  Thanks.
>>> Has this branch been pulled in yet? My patch, for one, is not even in
>>> linux-next even though it had been intended for 2.6.28 at the time.
>>
>> Jeff, have you pulled it or taken patches from it?
>>
>> Thanks.
> 
> I sent an email a couple days ago, asking what was pending (or thought I
> did...).  I haven't pulled from #tj-upstream but I ack'd a big patchset
> that I assume is included in there.

Eh... those are different ones.  #tj-upstream contained patches
scheduled for 2.6.28 (mostly fixes and stuff which were around for quite
some time).  I'll resend the patchset.

Thanks.

-- 
tejun