* Re: [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
@ 2014-06-04 14:51 scameron
2014-06-04 15:10 ` Julia Lawall
2014-06-04 16:55 ` Jens Axboe
0 siblings, 2 replies; 6+ messages in thread
From: scameron @ 2014-06-04 14:51 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-kernel, axboe
> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> return a larger number than the maximum position argument if that position
> is not a multiple of BITS_PER_LONG.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression e1,e2,e3;
> statement S1,S2;
> @@
>
> e1 = find_first_zero_bit(e2,e3)
> ...
> if (e1
> - ==
> + >=
> e3)
> S1 else S2
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> drivers/block/cciss.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
>
> do {
> i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> - if (i == h->nr_cmds)
> + if (i >= h->nr_cmds)
> return NULL;
> } while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
> c = h->cmd_pool + i;
Thanks. Ack.
You can add
Reviewed-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
to this patch if you want.
You might consider adding "Cc: stable@vger.kernel.org" into the
sign-off area as well.
-- steve
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
2014-06-04 14:51 [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit scameron
@ 2014-06-04 15:10 ` Julia Lawall
2014-06-04 16:55 ` Jens Axboe
1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2014-06-04 15:10 UTC (permalink / raw)
To: scameron; +Cc: linux-kernel, axboe
On Wed, 4 Jun 2014, scameron@beardog.cce.hp.com wrote:
> > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > return a larger number than the maximum position argument if that position
> > is not a multiple of BITS_PER_LONG.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression e1,e2,e3;
> > statement S1,S2;
> > @@
> >
> > e1 = find_first_zero_bit(e2,e3)
> > ...
> > if (e1
> > - ==
> > + >=
> > e3)
> > S1 else S2
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > drivers/block/cciss.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
> > --- a/drivers/block/cciss.c
> > +++ b/drivers/block/cciss.c
> > @@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
> >
> > do {
> > i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> > - if (i == h->nr_cmds)
> > + if (i >= h->nr_cmds)
> > return NULL;
> > } while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
> > c = h->cmd_pool + i;
>
>
> Thanks. Ack.
>
> You can add
>
> Reviewed-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>
> to this patch if you want.
>
> You might consider adding "Cc: stable@vger.kernel.org" into the
> sign-off area as well.
Likewise here, the change is not needed.
julia
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
2014-06-04 14:51 [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit scameron
2014-06-04 15:10 ` Julia Lawall
@ 2014-06-04 16:55 ` Jens Axboe
2014-06-04 16:59 ` Julia Lawall
1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2014-06-04 16:55 UTC (permalink / raw)
To: scameron, Julia Lawall; +Cc: linux-kernel
On 06/04/2014 08:51 AM, scameron@beardog.cce.hp.com wrote:
>> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
>> return a larger number than the maximum position argument if that position
>> is not a multiple of BITS_PER_LONG.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> expression e1,e2,e3;
>> statement S1,S2;
>> @@
>>
>> e1 = find_first_zero_bit(e2,e3)
>> ...
>> if (e1
>> - ==
>> + >=
>> e3)
>> S1 else S2
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> ---
>> drivers/block/cciss.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
>> --- a/drivers/block/cciss.c
>> +++ b/drivers/block/cciss.c
>> @@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
>>
>> do {
>> i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>> - if (i == h->nr_cmds)
>> + if (i >= h->nr_cmds)
>> return NULL;
>> } while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
>> c = h->cmd_pool + i;
>
>
> Thanks. Ack.
>
> You can add
>
> Reviewed-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>
> to this patch if you want.
>
> You might consider adding "Cc: stable@vger.kernel.org" into the
> sign-off area as well.
There are two such instances in cciss.c, btw.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
2014-06-04 16:55 ` Jens Axboe
@ 2014-06-04 16:59 ` Julia Lawall
2014-06-04 17:02 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2014-06-04 16:59 UTC (permalink / raw)
To: Jens Axboe; +Cc: scameron, linux-kernel
On Wed, 4 Jun 2014, Jens Axboe wrote:
> On 06/04/2014 08:51 AM, scameron@beardog.cce.hp.com wrote:
> >> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> >> return a larger number than the maximum position argument if that position
> >> is not a multiple of BITS_PER_LONG.
> >>
> >> The semantic match that finds this problem is as follows:
> >> (http://coccinelle.lip6.fr/)
> >>
> >> // <smpl>
> >> @@
> >> expression e1,e2,e3;
> >> statement S1,S2;
> >> @@
> >>
> >> e1 = find_first_zero_bit(e2,e3)
> >> ...
> >> if (e1
> >> - ==
> >> + >=
> >> e3)
> >> S1 else S2
> >> // </smpl>
> >>
> >> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >>
> >> ---
> >> drivers/block/cciss.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
> >> --- a/drivers/block/cciss.c
> >> +++ b/drivers/block/cciss.c
> >> @@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
> >>
> >> do {
> >> i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> >> - if (i == h->nr_cmds)
> >> + if (i >= h->nr_cmds)
> >> return NULL;
> >> } while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
> >> c = h->cmd_pool + i;
> >
> >
> > Thanks. Ack.
> >
> > You can add
> >
> > Reviewed-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> >
> > to this patch if you want.
> >
> > You might consider adding "Cc: stable@vger.kernel.org" into the
> > sign-off area as well.
>
> There are two such instances in cciss.c, btw.
Actually, there seem to be three, and I didn't find the other two because
the assignment is inlined into the test. But the patch isn't needed
anyway, because it turns out that the result never goes over the bound
value.
thanks,
julia
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
2014-06-04 16:59 ` Julia Lawall
@ 2014-06-04 17:02 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2014-06-04 17:02 UTC (permalink / raw)
To: Julia Lawall; +Cc: scameron, linux-kernel
On 06/04/2014 10:59 AM, Julia Lawall wrote:
>
>
> On Wed, 4 Jun 2014, Jens Axboe wrote:
>
>> On 06/04/2014 08:51 AM, scameron@beardog.cce.hp.com wrote:
>>>> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
>>>> return a larger number than the maximum position argument if that position
>>>> is not a multiple of BITS_PER_LONG.
>>>>
>>>> The semantic match that finds this problem is as follows:
>>>> (http://coccinelle.lip6.fr/)
>>>>
>>>> // <smpl>
>>>> @@
>>>> expression e1,e2,e3;
>>>> statement S1,S2;
>>>> @@
>>>>
>>>> e1 = find_first_zero_bit(e2,e3)
>>>> ...
>>>> if (e1
>>>> - ==
>>>> + >=
>>>> e3)
>>>> S1 else S2
>>>> // </smpl>
>>>>
>>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>>>
>>>> ---
>>>> drivers/block/cciss.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
>>>> --- a/drivers/block/cciss.c
>>>> +++ b/drivers/block/cciss.c
>>>> @@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
>>>>
>>>> do {
>>>> i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>>>> - if (i == h->nr_cmds)
>>>> + if (i >= h->nr_cmds)
>>>> return NULL;
>>>> } while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
>>>> c = h->cmd_pool + i;
>>>
>>>
>>> Thanks. Ack.
>>>
>>> You can add
>>>
>>> Reviewed-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>>>
>>> to this patch if you want.
>>>
>>> You might consider adding "Cc: stable@vger.kernel.org" into the
>>> sign-off area as well.
>>
>> There are two such instances in cciss.c, btw.
>
> Actually, there seem to be three, and I didn't find the other two because
> the assignment is inlined into the test. But the patch isn't needed
> anyway, because it turns out that the result never goes over the bound
> value.
I have always defensively programmed it, but it would make a shitty API
if it did.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/10] use safer test on the result of find_first_zero_bit
@ 2014-06-04 9:07 Julia Lawall
2014-06-04 9:07 ` [PATCH 7/10] cciss: " Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2014-06-04 9:07 UTC (permalink / raw)
To: linux-rdma
Cc: kernel-janitors, linux-fbdev, linux-sh, linux-kernel, ath10k,
linux-wireless, netdev, devel, iss_storagedev, linux-scsi,
linux-s390, adi-buildroot-devel
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
2014-06-04 9:07 [PATCH 0/10] " Julia Lawall
@ 2014-06-04 9:07 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2014-06-04 9:07 UTC (permalink / raw)
To: Mike Miller; +Cc: kernel-janitors, iss_storagedev, linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression e1,e2,e3;
statement S1,S2;
@@
e1 = find_first_zero_bit(e2,e3)
...
if (e1
- ==
+ >=
e3)
S1 else S2
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/block/cciss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/block/cciss.c b/drivers/block/cciss.c
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -980,7 +980,7 @@ static CommandList_struct *cmd_alloc(ctl
do {
i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
- if (i == h->nr_cmds)
+ if (i >= h->nr_cmds)
return NULL;
} while (test_and_set_bit(i, h->cmd_pool_bits) != 0);
c = h->cmd_pool + i;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-04 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-04 14:51 [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit scameron
2014-06-04 15:10 ` Julia Lawall
2014-06-04 16:55 ` Jens Axboe
2014-06-04 16:59 ` Julia Lawall
2014-06-04 17:02 ` Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2014-06-04 9:07 [PATCH 0/10] " Julia Lawall
2014-06-04 9:07 ` [PATCH 7/10] cciss: " Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox