Skip to content
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

Fix recent coverity-reported issues #3392

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

matt335672
Copy link
Member

Issue report via email was:-

Hi,

Please find the latest report on new defect(s) introduced to neutrinolabs/xrdp found with Coverity Scan.

3 new defect(s) introduced to neutrinolabs/xrdp found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 3 of 3 defect(s)


** CID 471853:  Resource leaks  (RESOURCE_LEAK)
/sesman/chansrv/chansrv_fuse.c: 2690 in xfuse_cb_opendir()


________________________________________________________________________________________________________
*** CID 471853:  Resource leaks  (RESOURCE_LEAK)
/sesman/chansrv/chansrv_fuse.c: 2690 in xfuse_cb_opendir()
2684             if ((xhandle = xfuse_handle_create()) == NULL)
2685             {
2686                 fuse_reply_err(req, ENOMEM);
2687             }
2688             else
2689             {
>>>     CID 471853:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by "xhandle->dir_handle" leaks it.
2690                 if ((xhandle->dir_handle = xfs_opendir(g_xfs, ino)) == NULL)
2691                 {
2692                     xfuse_handle_delete(xhandle);
2693                     fuse_reply_err(req, ENOMEM);
2694                 }
2695                 else

** CID 471852:    (RESOURCE_LEAK)
/xrdpapi/xrdpapi.c: 182 in WTSVirtualChannelOpenEx()
/xrdpapi/xrdpapi.c: 158 in WTSVirtualChannelOpenEx()
/xrdpapi/xrdpapi.c: 167 in WTSVirtualChannelOpenEx()


________________________________________________________________________________________________________
*** CID 471852:    (RESOURCE_LEAK)
/xrdpapi/xrdpapi.c: 182 in WTSVirtualChannelOpenEx()
176                   chan_name_bytes, bytes, pVirtualName);
177     
178         connect_data = (char *) calloc(bytes, 1);
179         if (connect_data == NULL)
180         {
181             LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: calloc failed");
>>>     CID 471852:    (RESOURCE_LEAK)
>>>     Freeing "wts" without freeing its handle field "fd" leaks the handle.
182             free(wts);
183             return NULL;
184         }
185     
186         connect_data[0] = (bytes >> 0) & 0xFF;
187         connect_data[1] = (bytes >> 8) & 0xFF;
/xrdpapi/xrdpapi.c: 158 in WTSVirtualChannelOpenEx()
152             {
153                 /* ok */
154             }
155             else
156             {
157                 LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: connect failed");
>>>     CID 471852:    (RESOURCE_LEAK)
>>>     Freeing "wts" without freeing its handle field "fd" leaks the handle.
158                 free(wts);
159                 return NULL;
160             }
161         }
162     
163         /* wait for connection to complete */
/xrdpapi/xrdpapi.c: 167 in WTSVirtualChannelOpenEx()
161         }
162     
163         /* wait for connection to complete */
164         if (!can_send(wts->fd, 500, 1))
165         {
166             LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: can_send failed");
>>>     CID 471852:    (RESOURCE_LEAK)
>>>     Freeing "wts" without freeing its handle field "fd" leaks the handle.
167             free(wts);
168             return NULL;
169         }
170     
171         chan_name_bytes = strlen(pVirtualName);
172         bytes = 4 + 4 + 4 + chan_name_bytes + 4;

** CID 471851:  Resource leaks  (RESOURCE_LEAK)
/sesman/chansrv/chansrv_fuse.c: 1172 in xfuse_devredir_cb_enum_dir_done()


________________________________________________________________________________________________________
*** CID 471851:  Resource leaks  (RESOURCE_LEAK)
/sesman/chansrv/chansrv_fuse.c: 1172 in xfuse_devredir_cb_enum_dir_done()
1166         }
1167         else
1168         {
1169             struct fuse_file_info *fi = &fip->fi;
1170             XFUSE_HANDLE *xhandle = xfuse_handle_create();
1171     
>>>     CID 471851:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by "xhandle->dir_handle" leaks it.
1172             if (xhandle == NULL
1173                     || (xhandle->dir_handle = xfs_opendir(g_xfs, fip->pinum)) == NULL)
1174             {
1175                 xfuse_handle_delete(xhandle);
1176                 fuse_reply_err(fip->req, ENOMEM);
1177             }

The errors in sesman/chansrv/chansrv_fuse.c appear to be false positives caused by allocating xhandle->dir_handle, and then casting xhandle to an integer in xfuse_handle_to_fuse_handle(), thus hiding xhandle->dir_handle

For both occurrences, the logic has been simplified and made the same, and a comment has been added to suppress the error.

The errors in sesman/chansrv/chansrv_fuse.c appear to be false positives
caused by allocating xhandle->dir_handle, and then
casting xhandle to an integer in xfuse_handle_to_fuse_handle(), thus
hiding xhandle->dir_handle

For both occurrences, the logic has been simplified and made the same,
and a comment has been added to suppress the error.
@metalefty
Copy link
Member

Yeah, I am about to work on this. Thank you.

@matt335672
Copy link
Member Author

I'll merge and we'll see what happens. I've not used the //coverity comment before, so I may well have it wrong.

@matt335672 matt335672 merged commit a0f1b65 into neutrinolabs:devel Jan 13, 2025
14 checks passed
@matt335672 matt335672 deleted the fix_cov_err branch January 13, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants