Skip to content

Coverity/static analysis issues with TCP interface code #9705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jsquyres opened this issue Nov 24, 2021 · 5 comments
Closed

Coverity/static analysis issues with TCP interface code #9705

jsquyres opened this issue Nov 24, 2021 · 5 comments
Assignees

Comments

@jsquyres
Copy link
Member

@drwootton @jjhursey Looks like we got some static analysis problems from 0b37cd2 (PR #9681). Can you fix?

Thanks!

________________________________________________________________________________________________________
*** CID 1494436:  Null pointer dereferences  (FORWARD_NULL)
/opal/mca/btl/tcp/btl_tcp_component.c: 758 in split_and_resolve()
752             }
753     
754             free(tmp);
755         }
756     
757         /* Mark the end of the interface name array with NULL */
   CID 1494436:  Null pointer dereferences  (FORWARD_NULL)
   Dereferencing null pointer "interfaces".
758         interfaces[interface_count] = NULL;
759         free(argv);
760         free(*orig_str);
761         *orig_str = opal_argv_join(interfaces, ',');
762         return interfaces;
763     }
** CID 1494435:    (RESOURCE_LEAK)
/opal/mca/btl/tcp/btl_tcp_component.c: 676 in split_and_resolve()
/opal/mca/btl/tcp/btl_tcp_component.c: 738 in split_and_resolve()
________________________________________________________________________________________________________
*** CID 1494435:    (RESOURCE_LEAK)
/opal/mca/btl/tcp/btl_tcp_component.c: 676 in split_and_resolve()
670                     }
671                 }
672                 if (n == interface_count) {
673                     opal_output_verbose(20,
674                                         opal_btl_base_framework.framework_output,
675                                         "btl: tcp: Using interface: %s ", argv[i]);
   CID 1494435:    (RESOURCE_LEAK)
   Failing to save or free storage allocated by "strdup(argv[i])" leaks it.
676                     opal_argv_append(&interface_count, &interfaces, strdup(argv[i]));
677                 }
678                 continue;
679             }
680     
681             /* Found a subnet notation.  Convert it to an IP
/opal/mca/btl/tcp/btl_tcp_component.c: 738 in split_and_resolve()
732                     if (n == interface_count) {
733                         opal_output_verbose(20,
734                                             opal_btl_base_framework.framework_output,
735                                             "btl: tcp: Found match: %s (%s)",
736                                             opal_net_get_hostname((struct sockaddr*) &if_inaddr),
737                                             if_name);
   CID 1494435:    (RESOURCE_LEAK)
   Failing to save or free storage allocated by "strdup(if_name)" leaks it.
738                         opal_argv_append(&interface_count, &interfaces, strdup(if_name));
739                     }
740                 }
741             }
742     
743             /* If we didn't find a match, keep trying */
@drwootton
Copy link
Contributor

I started looking at this. I need to discuss with @jjhursey to figure out how to deal with the resource leak complaints.

@jsquyres
Copy link
Member Author

Thanks @drwootton!

@bosilca
Copy link
Member

bosilca commented Nov 24, 2021

opal_argv_append already strdup the appended string, so you don't have to explicitly call strdup. The solution is to just remove the strdup from the opal_argv_append calls.

@drwootton
Copy link
Contributor

I didn't realize the strdup was taken care of already. I also know what I need to do to take care of the potential null pointer dereference, so I'll take care of this all on Monday.

Thanks.

jjhursey added a commit that referenced this issue Dec 1, 2021
Resolve Coverity problems reported in issue #9705
@jjhursey
Copy link
Member

jjhursey commented Dec 1, 2021

PR #9705 should resolve the coverity issues. We will watch the coverity report now that this is merged into master.

@jjhursey jjhursey closed this as completed Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants