diff mbox series

[ovs-dev] conntrack: Fix NULL pointer dereference.

Message ID 1584486741-79152-1-git-send-email-u9012063@gmail.com
State Accepted
Commit 9a8a18f9fa27f72cfdb9da75f10bffc9ab621e33
Headers show
Series [ovs-dev] conntrack: Fix NULL pointer dereference. | expand

Commit Message

William Tu March 17, 2020, 11:12 p.m. UTC 
Coverity CID 279957 reports NULL pointer derefence when
'conn' is NULL and calling ct_print_conn_info.

Cc: Usman Ansari <uansari@vmware.com>
Signed-off-by: William Tu <u9012063@gmail.com>
---
 lib/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ilya Maximets March 18, 2020, 12:49 p.m. UTC | #1 
On 3/18/20 12:12 AM, William Tu wrote:
> Coverity CID 279957 reports NULL pointer derefence when
> 'conn' is NULL and calling ct_print_conn_info.
> 
> Cc: Usman Ansari <uansari@vmware.com>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  lib/conntrack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index ff5a89457c0a..001a37ff6aff 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>              if (!conn) {
>                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
>                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
> -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
>                  free(log_msg);
>                  return;
>              }
> 

Hi.

This issue is addressed as part of the following patch:
  https://patchwork.ozlabs.org/patch/1249513/
I'm not sure if we need to split it and fix this issue separately.
Thoughts?

Best regards, Ilya Maximets.
Ben Pfaff March 18, 2020, 3:23 p.m. UTC | #2 
On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
> On 3/18/20 12:12 AM, William Tu wrote:
> > Coverity CID 279957 reports NULL pointer derefence when
> > 'conn' is NULL and calling ct_print_conn_info.
> > 
> > Cc: Usman Ansari <uansari@vmware.com>
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> >  lib/conntrack.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index ff5a89457c0a..001a37ff6aff 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
> >              if (!conn) {
> >                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> >                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
> > -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> > +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
> >                  free(log_msg);
> >                  return;
> >              }
> > 
> 
> Hi.
> 
> This issue is addressed as part of the following patch:
>   https://patchwork.ozlabs.org/patch/1249513/
> I'm not sure if we need to split it and fix this issue separately.
> Thoughts?

It seems like a separate issue to me, just located in nearby code.
William Tu March 19, 2020, 3:02 p.m. UTC | #3 
On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
> > On 3/18/20 12:12 AM, William Tu wrote:
> > > Coverity CID 279957 reports NULL pointer derefence when
> > > 'conn' is NULL and calling ct_print_conn_info.
> > >
> > > Cc: Usman Ansari <uansari@vmware.com>
> > > Signed-off-by: William Tu <u9012063@gmail.com>
> > > ---
> > >  lib/conntrack.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > > index ff5a89457c0a..001a37ff6aff 100644
> > > --- a/lib/conntrack.c
> > > +++ b/lib/conntrack.c
> > > @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
> > >              if (!conn) {
> > >                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> > >                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
> > > -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> > > +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
> > >                  free(log_msg);
> > >                  return;
> > >              }
> > >
> >
> > Hi.
> >
> > This issue is addressed as part of the following patch:
> >   https://patchwork.ozlabs.org/patch/1249513/
> > I'm not sure if we need to split it and fix this issue separately.
> > Thoughts?
>
> It seems like a separate issue to me, just located in nearby code.

so split and fix separately?
William
Dumitru Ceara March 19, 2020, 4:31 p.m. UTC | #4 
On 3/19/20 4:02 PM, William Tu wrote:
> On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff <blp@ovn.org> wrote:
>>
>> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
>>> On 3/18/20 12:12 AM, William Tu wrote:
>>>> Coverity CID 279957 reports NULL pointer derefence when
>>>> 'conn' is NULL and calling ct_print_conn_info.
>>>>
>>>> Cc: Usman Ansari <uansari@vmware.com>
>>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>>> ---
>>>>  lib/conntrack.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>>>> index ff5a89457c0a..001a37ff6aff 100644
>>>> --- a/lib/conntrack.c
>>>> +++ b/lib/conntrack.c
>>>> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>>>>              if (!conn) {
>>>>                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
>>>>                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
>>>> -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
>>>> +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
>>>>                  free(log_msg);
>>>>                  return;
>>>>              }
>>>>
>>>
>>> Hi.
>>>
>>> This issue is addressed as part of the following patch:
>>>   https://patchwork.ozlabs.org/patch/1249513/
>>> I'm not sure if we need to split it and fix this issue separately.
>>> Thoughts?
>>
>> It seems like a separate issue to me, just located in nearby code.
> 
> so split and fix separately?
> William

Hi William,

I'll send a v3 of https://patchwork.ozlabs.org/patch/1249513/ and remove
the conflict in my patch. Better to keep fixes separate indeed.

Thanks,
Dumitru
William Tu March 19, 2020, 7:06 p.m. UTC | #5 
On Thu, Mar 19, 2020 at 9:31 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 3/19/20 4:02 PM, William Tu wrote:
> > On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff <blp@ovn.org> wrote:
> >>
> >> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
> >>> On 3/18/20 12:12 AM, William Tu wrote:
> >>>> Coverity CID 279957 reports NULL pointer derefence when
> >>>> 'conn' is NULL and calling ct_print_conn_info.
> >>>>
> >>>> Cc: Usman Ansari <uansari@vmware.com>
> >>>> Signed-off-by: William Tu <u9012063@gmail.com>
> >>>> ---
> >>>>  lib/conntrack.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/conntrack.c b/lib/conntrack.c
> >>>> index ff5a89457c0a..001a37ff6aff 100644
> >>>> --- a/lib/conntrack.c
> >>>> +++ b/lib/conntrack.c
> >>>> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
> >>>>              if (!conn) {
> >>>>                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> >>>>                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
> >>>> -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> >>>> +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
> >>>>                  free(log_msg);
> >>>>                  return;
> >>>>              }
> >>>>
> >>>
> >>> Hi.
> >>>
> >>> This issue is addressed as part of the following patch:
> >>>   https://patchwork.ozlabs.org/patch/1249513/
> >>> I'm not sure if we need to split it and fix this issue separately.
> >>> Thoughts?
> >>
> >> It seems like a separate issue to me, just located in nearby code.
> >
> > so split and fix separately?
> > William
>
> Hi William,
>
> I'll send a v3 of https://patchwork.ozlabs.org/patch/1249513/ and remove
> the conflict in my patch. Better to keep fixes separate indeed.
>
> Thanks,
> Dumitru
>
OK thanks!
William
Dumitru Ceara March 19, 2020, 7:33 p.m. UTC | #6 
On 3/18/20 12:12 AM, William Tu wrote:
> Coverity CID 279957 reports NULL pointer derefence when
> 'conn' is NULL and calling ct_print_conn_info.
> 
> Cc: Usman Ansari <uansari@vmware.com>
> Signed-off-by: William Tu <u9012063@gmail.com>

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru

> ---
>  lib/conntrack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index ff5a89457c0a..001a37ff6aff 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>              if (!conn) {
>                  pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
>                  char *log_msg = xasprintf("Missing master conn %p", rev_conn);
> -                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> +                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
>                  free(log_msg);
>                  return;
>              }
>
William Tu March 20, 2020, 12:04 a.m. UTC | #7 
On Thu, Mar 19, 2020 at 08:33:24PM +0100, Dumitru Ceara wrote:
> On 3/18/20 12:12 AM, William Tu wrote:
> > Coverity CID 279957 reports NULL pointer derefence when
> > 'conn' is NULL and calling ct_print_conn_info.
> > 
> > Cc: Usman Ansari <uansari@vmware.com>
> > Signed-off-by: William Tu <u9012063@gmail.com>
> 
> Acked-by: Dumitru Ceara <dceara@redhat.com>
> 
> Thanks,
> Dumitru

Thanks
Applied to master and 2.13
Willam
Ilya Maximets March 20, 2020, 9:56 a.m. UTC | #8 
On 3/20/20 1:04 AM, William Tu wrote:
> On Thu, Mar 19, 2020 at 08:33:24PM +0100, Dumitru Ceara wrote:
>> On 3/18/20 12:12 AM, William Tu wrote:
>>> Coverity CID 279957 reports NULL pointer derefence when
>>> 'conn' is NULL and calling ct_print_conn_info.
>>>
>>> Cc: Usman Ansari <uansari@vmware.com>
>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>
>> Acked-by: Dumitru Ceara <dceara@redhat.com>
>>
>> Thanks,
>> Dumitru
> 
> Thanks
> Applied to master and 2.13

Could you, please, backport to 2.12 too?

Best regards, Ilya Maximets.
William Tu March 20, 2020, 2:24 p.m. UTC | #9 
On Fri, Mar 20, 2020 at 2:56 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 3/20/20 1:04 AM, William Tu wrote:
> > On Thu, Mar 19, 2020 at 08:33:24PM +0100, Dumitru Ceara wrote:
> >> On 3/18/20 12:12 AM, William Tu wrote:
> >>> Coverity CID 279957 reports NULL pointer derefence when
> >>> 'conn' is NULL and calling ct_print_conn_info.
> >>>
> >>> Cc: Usman Ansari <uansari@vmware.com>
> >>> Signed-off-by: William Tu <u9012063@gmail.com>
> >>
> >> Acked-by: Dumitru Ceara <dceara@redhat.com>
> >>
> >> Thanks,
> >> Dumitru
> >
> > Thanks
> > Applied to master and 2.13
>
> Could you, please, backport to 2.12 too?
>
Done.
Thanks
William
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index ff5a89457c0a..001a37ff6aff 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1302,7 +1302,7 @@  process_one(struct conntrack *ct, struct dp_packet *pkt,
             if (!conn) {
                 pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
                 char *log_msg = xasprintf("Missing master conn %p", rev_conn);
-                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
+                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
                 free(log_msg);
                 return;
             }