* [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
@ 2013-06-12 13:55 Hal Rosenstock
[not found] ` <51B87DD4.9040005-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Hal Rosenstock @ 2013-06-12 13:55 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
From: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Buffer may not have a null terminator if the source string's length is
equal to the buffer size.
Signed-off-by: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
src/ibstat.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/ibstat.c b/src/ibstat.c
index 665bb0a..acb9e8e 100644
--- a/src/ibstat.c
+++ b/src/ibstat.c
@@ -279,6 +279,7 @@ int main(int argc, char *argv[])
int dev_port = -1;
int n, i;
+ memset(names, 0, sizeof(names[0][0] * UMAD_MAX_DEVICES * UMAD_CA_NAME_LEN));
const struct ibdiag_opt opts[] = {
{"list_of_cas", 'l', 0, NULL, "list all IB devices"},
{"short", 's', 0, NULL, "short output"},
@@ -314,7 +315,7 @@ int main(int argc, char *argv[])
if (i >= n)
IBPANIC("'%s' IB device can't be found", argv[0]);
- strncpy(names[i], argv[0], sizeof names[i]);
+ strncpy(names[i], argv[0], sizeof names[i]-1);
n = 1;
}
--
1.7.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
[not found] ` <51B87DD4.9040005-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
@ 2013-06-12 21:19 ` Ira Weiny
[not found] ` <20130612141948.c4760f2856d2e4435be2b0a7-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Ira Weiny @ 2013-06-12 21:19 UTC (permalink / raw)
To: Hal Rosenstock
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
On Wed, 12 Jun 2013 09:55:32 -0400
Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> From: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
>
> Buffer may not have a null terminator if the source string's length is
> equal to the buffer size.
Good catch, but I think this is an issue in umad_get_cas_names!
>
> Signed-off-by: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
> src/ibstat.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/ibstat.c b/src/ibstat.c
> index 665bb0a..acb9e8e 100644
> --- a/src/ibstat.c
> +++ b/src/ibstat.c
> @@ -279,6 +279,7 @@ int main(int argc, char *argv[])
> int dev_port = -1;
> int n, i;
>
> + memset(names, 0, sizeof(names[0][0] * UMAD_MAX_DEVICES * UMAD_CA_NAME_LEN));
I don't think you need this as it will not fix umad_get_cas_names.
> const struct ibdiag_opt opts[] = {
> {"list_of_cas", 'l', 0, NULL, "list all IB devices"},
> {"short", 's', 0, NULL, "short output"},
> @@ -314,7 +315,7 @@ int main(int argc, char *argv[])
> if (i >= n)
> IBPANIC("'%s' IB device can't be found", argv[0]);
>
> - strncpy(names[i], argv[0], sizeof names[i]);
> + strncpy(names[i], argv[0], sizeof names[i]-1);
This is actually dead code. IBPANIC exits, if your linking to libibmad. Do you have a different IBPANIC which does not exit? Do you have a use case which hits this bug?
[root@iqa-136 sbin]# ./ibstat myverylongca_name_0123456789_0123456789
ibpanic: [9262] main: 'myverylongca_name_0123456789_0123456789' IB device can't be found: Success
[root@iqa-136 sbin]# ./ibstat notfoundca
ibpanic: [9273] main: 'notfoundca' IB device can't be found: Success
Ira
> n = 1;
> }
>
> --
> 1.7.8.2
>
--
Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
[not found] ` <20130612141948.c4760f2856d2e4435be2b0a7-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2013-06-12 22:03 ` Hal Rosenstock
[not found] ` <51B8F03C.8010004-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Hal Rosenstock @ 2013-06-12 22:03 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
On 6/12/2013 5:19 PM, Ira Weiny wrote:
> On Wed, 12 Jun 2013 09:55:32 -0400
> Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>
>> From: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
>>
>> Buffer may not have a null terminator if the source string's length is
>> equal to the buffer size.
>
> Good catch, but I think this is an issue in umad_get_cas_names!
>
>>
>> Signed-off-by: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
>> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>> src/ibstat.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/ibstat.c b/src/ibstat.c
>> index 665bb0a..acb9e8e 100644
>> --- a/src/ibstat.c
>> +++ b/src/ibstat.c
>> @@ -279,6 +279,7 @@ int main(int argc, char *argv[])
>> int dev_port = -1;
>> int n, i;
>>
>> + memset(names, 0, sizeof(names[0][0] * UMAD_MAX_DEVICES * UMAD_CA_NAME_LEN));
>
> I don't think you need this as it will not fix umad_get_cas_names.
The proper memset is already in the tree.
>> const struct ibdiag_opt opts[] = {
>> {"list_of_cas", 'l', 0, NULL, "list all IB devices"},
>> {"short", 's', 0, NULL, "short output"},
>> @@ -314,7 +315,7 @@ int main(int argc, char *argv[])
>> if (i >= n)
>> IBPANIC("'%s' IB device can't be found", argv[0]);
>>
>> - strncpy(names[i], argv[0], sizeof names[i]);
>> + strncpy(names[i], argv[0], sizeof names[i]-1);
>
> This is actually dead code. IBPANIC exits, if your linking to libibmad. Do you have a different IBPANIC which does not exit? Do you have a use case which hits this bug?
>
> [root@iqa-136 sbin]# ./ibstat myverylongca_name_0123456789_0123456789
> ibpanic: [9262] main: 'myverylongca_name_0123456789_0123456789' IB device can't be found: Success
> [root@iqa-136 sbin]# ./ibstat notfoundca
> ibpanic: [9273] main: 'notfoundca' IB device can't be found: Success
I think the change is to quiet a Coverity detected error but Dan is the
definitive source for this change.
-- Hal
> Ira
>
>> n = 1;
>> }
>>
>> --
>> 1.7.8.2
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
[not found] ` <51B8F03C.8010004-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
@ 2013-06-12 22:06 ` Hal Rosenstock
[not found] ` <51B8F0E9.90508-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Hal Rosenstock @ 2013-06-12 22:06 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
On 6/12/2013 6:03 PM, Hal Rosenstock wrote:
> On 6/12/2013 5:19 PM, Ira Weiny wrote:
>> On Wed, 12 Jun 2013 09:55:32 -0400
>> Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>>
>>> From: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
>>>
>>> Buffer may not have a null terminator if the source string's length is
>>> equal to the buffer size.
>>
>> Good catch, but I think this is an issue in umad_get_cas_names!
>>
>>>
>>> Signed-off-by: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
>>> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> ---
>>> src/ibstat.c | 3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/ibstat.c b/src/ibstat.c
>>> index 665bb0a..acb9e8e 100644
>>> --- a/src/ibstat.c
>>> +++ b/src/ibstat.c
>>> @@ -279,6 +279,7 @@ int main(int argc, char *argv[])
>>> int dev_port = -1;
>>> int n, i;
>>>
>>> + memset(names, 0, sizeof(names[0][0] * UMAD_MAX_DEVICES * UMAD_CA_NAME_LEN));
>>
>> I don't think you need this as it will not fix umad_get_cas_names.
>
> The proper memset is already in the tree.
It isn't in the tree; I was looking at modified source...
>>> const struct ibdiag_opt opts[] = {
>>> {"list_of_cas", 'l', 0, NULL, "list all IB devices"},
>>> {"short", 's', 0, NULL, "short output"},
>>> @@ -314,7 +315,7 @@ int main(int argc, char *argv[])
>>> if (i >= n)
>>> IBPANIC("'%s' IB device can't be found", argv[0]);
>>>
>>> - strncpy(names[i], argv[0], sizeof names[i]);
>>> + strncpy(names[i], argv[0], sizeof names[i]-1);
>>
>> This is actually dead code. IBPANIC exits, if your linking to libibmad. Do you have a different IBPANIC which does not exit? Do you have a use case which hits this bug?
>>
>> [root@iqa-136 sbin]# ./ibstat myverylongca_name_0123456789_0123456789
>> ibpanic: [9262] main: 'myverylongca_name_0123456789_0123456789' IB device can't be found: Success
>> [root@iqa-136 sbin]# ./ibstat notfoundca
>> ibpanic: [9273] main: 'notfoundca' IB device can't be found: Success
>
> I think the change is to quiet a Coverity detected error but Dan is the
> definitive source for this change.
>
> -- Hal
>
>> Ira
>>
>>> n = 1;
>>> }
>>>
>>> --
>>> 1.7.8.2
>>>
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
[not found] ` <51B8F0E9.90508-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
@ 2013-06-12 22:17 ` Weiny, Ira
[not found] ` <2807E5FD2F6FDA4886F6618EAC48510E0207E124-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Weiny, Ira @ 2013-06-12 22:17 UTC (permalink / raw)
To: Hal Rosenstock
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
> -----Original Message-----
> From: Hal Rosenstock [mailto:hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org]
> Subject: Re: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-
> terminated
>
> On 6/12/2013 6:03 PM, Hal Rosenstock wrote:
> > On 6/12/2013 5:19 PM, Ira Weiny wrote:
> >> On Wed, 12 Jun 2013 09:55:32 -0400
> >> Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> >>
> >>> From: Dan Ben Yosef <danby-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
> >>>
> >>>
> >>> + memset(names, 0, sizeof(names[0][0] * UMAD_MAX_DEVICES *
> >>> +UMAD_CA_NAME_LEN));
> >>
> >> I don't think you need this as it will not fix umad_get_cas_names.
> >
> > The proper memset is already in the tree.
>
> It isn't in the tree; I was looking at modified source...
Sounds like that should be pushed upstream! ;-)
>
> >>> const struct ibdiag_opt opts[] = {
> >>> {"list_of_cas", 'l', 0, NULL, "list all IB devices"},
> >>> {"short", 's', 0, NULL, "short output"}, @@ -314,7 +315,7 @@
> int
> >>> main(int argc, char *argv[])
> >>> if (i >= n)
> >>> IBPANIC("'%s' IB device can't be found", argv[0]);
> >>>
> >>> - strncpy(names[i], argv[0], sizeof names[i]);
> >>> + strncpy(names[i], argv[0], sizeof names[i]-1);
> >>
> >> This is actually dead code. IBPANIC exits, if your linking to libibmad. Do
> you have a different IBPANIC which does not exit? Do you have a use case
> which hits this bug?
> >>
> >> [root@iqa-136 sbin]# ./ibstat
> myverylongca_name_0123456789_0123456789
> >> ibpanic: [9262] main: 'myverylongca_name_0123456789_0123456789' IB
> >> device can't be found: Success
> >> [root@iqa-136 sbin]# ./ibstat notfoundca
> >> ibpanic: [9273] main: 'notfoundca' IB device can't be found: Success
> >
> > I think the change is to quiet a Coverity detected error but Dan is
> > the definitive source for this change.
I think we should remove the dead code and that should fix Coverity. I thought it detected dead code. I wonder if I missed something?
Ira
> >
> > -- Hal
> >
> >> Ira
> >>
> >>> n = 1;
> >>> }
> >>>
> >>> --
> >>> 1.7.8.2
> >>>
> >>
> >>
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated
[not found] ` <2807E5FD2F6FDA4886F6618EAC48510E0207E124-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2013-06-12 23:21 ` Weiny, Ira
0 siblings, 0 replies; 6+ messages in thread
From: Weiny, Ira @ 2013-06-12 23:21 UTC (permalink / raw)
To: Weiny, Ira, Hal Rosenstock
Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org),
Dan Ben-Yosef
> -----Original Message-----
> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> Subject: RE: [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-
> terminated
>
> > >
> > > I think the change is to quiet a Coverity detected error but Dan is
> > > the definitive source for this change.
>
> I think we should remove the dead code and that should fix Coverity. I
> thought it detected dead code. I wonder if I missed something?
That is not dead code. I misread where the break jumps to.
However, the code is redundant and there is a bug.[*]
16:16:51 > ibstat -l
mlx4_0
mlx4_1
16:17:57 > ibstat -l mlx4_1
mlx4_0
The intent was to copy argv[0] into names[0] and set n=1. But that is not happening...
Ira
[*] not that the above is really a valid use case.
>
> Ira
>
> > >
> > > -- Hal
> > >
> > >> Ira
> > >>
> > >>> n = 1;
> > >>> }
> > >>>
> > >>> --
> > >>> 1.7.8.2
> > >>>
> > >>
> > >>
> > >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the
> body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-12 23:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-12 13:55 [PATCH infiniband-diags 2/6] ibstat.c: fix buffer-not-null-terminated Hal Rosenstock
[not found] ` <51B87DD4.9040005-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-06-12 21:19 ` Ira Weiny
[not found] ` <20130612141948.c4760f2856d2e4435be2b0a7-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2013-06-12 22:03 ` Hal Rosenstock
[not found] ` <51B8F03C.8010004-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-06-12 22:06 ` Hal Rosenstock
[not found] ` <51B8F0E9.90508-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-06-12 22:17 ` Weiny, Ira
[not found] ` <2807E5FD2F6FDA4886F6618EAC48510E0207E124-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-06-12 23:21 ` Weiny, Ira
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.