* [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; 7+ 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] 7+ messages in thread
* [PATCH 7/10] cciss: use safer test on the result of find_first_zero_bit
@ 2014-06-04 9:07 ` Julia Lawall
0 siblings, 0 replies; 7+ 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] 7+ messages in thread
* 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2014-06-04 17:02 UTC | newest]
Thread overview: 7+ 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
2014-06-04 9:07 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.