Fix Coverity warnings
Two problems were found:- 1) A useless test in scp_list.c - testing an unsigned int was >= 0. 2) Flow control issues in scp.c:scp_get_connect_session_response() meant that file descriptors could be leaked. A helper function has been used to simplify the code.
This commit is contained in:
+75
-42
@@ -607,6 +607,58 @@ scp_send_connect_session_response(struct trans *trans,
|
||||
return rv;
|
||||
}
|
||||
|
||||
/*****************************************************************************/
|
||||
/**
|
||||
* Helper function to get the file descriptors for a connect
|
||||
*
|
||||
* @param trans SCP trans
|
||||
* @param[out] display_fd Display server file descriptor
|
||||
* @param[out] chan_fd Chansrv file descriptor
|
||||
* @return != 0 for error
|
||||
*
|
||||
* This wrapper is nneded as libipm doesn't currently guarantee to
|
||||
* handle received file descriptors well if an error is encountered
|
||||
* mid-message.
|
||||
*
|
||||
* If an error is returned, some file descriptors may be valid.
|
||||
*/
|
||||
static int
|
||||
get_connect_session_response_fds(struct trans *trans,
|
||||
int *display_fd,
|
||||
int *chan_fd)
|
||||
{
|
||||
int rv;
|
||||
int fd_present;
|
||||
|
||||
// Read the display server file descriptor and guard
|
||||
if ((rv = libipm_msg_in_parse(trans, "b", &fd_present)) != 0)
|
||||
{
|
||||
return rv;
|
||||
}
|
||||
if (fd_present)
|
||||
{
|
||||
if ((rv = libipm_msg_in_parse(trans, "h", display_fd)) != 0)
|
||||
{
|
||||
return rv;
|
||||
}
|
||||
}
|
||||
|
||||
// Read the chansrv file descriptor and guard
|
||||
if ((rv = libipm_msg_in_parse(trans, "b", &fd_present)) != 0)
|
||||
{
|
||||
return rv;
|
||||
}
|
||||
if (fd_present)
|
||||
{
|
||||
if ((rv = libipm_msg_in_parse(trans, "h", chan_fd)) != 0)
|
||||
{
|
||||
return rv;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*****************************************************************************/
|
||||
|
||||
int
|
||||
@@ -615,59 +667,40 @@ scp_get_connect_session_response(struct trans *trans,
|
||||
int *display_fd,
|
||||
int *chan_fd)
|
||||
{
|
||||
int fd_present;
|
||||
int got_display_fd = 0;
|
||||
int got_chan_fd = 0;
|
||||
|
||||
int rv;
|
||||
/* Intermediate values */
|
||||
int32_t i_status;
|
||||
int rv = libipm_msg_in_parse( trans, "i", &i_status);
|
||||
|
||||
// Read the X11 file descriptor
|
||||
if (rv == 0 && libipm_msg_in_parse(trans, "b", &fd_present) == 0)
|
||||
/* Set the returned FDs to nonsensical values to stop valid
|
||||
* FDs getting clobbered */
|
||||
*display_fd = -1;
|
||||
*chan_fd = -1;
|
||||
|
||||
if ((rv = libipm_msg_in_parse( trans, "i", &i_status)) == 0)
|
||||
{
|
||||
if (!fd_present)
|
||||
// Us a helper function to get the file descriptors as this
|
||||
// makes flow control easier.
|
||||
rv = get_connect_session_response_fds(trans, display_fd, chan_fd);
|
||||
if (rv == 0)
|
||||
{
|
||||
// Caller didn't send display_fd
|
||||
*display_fd = -1;
|
||||
*status = (enum scp_sconnect_status)i_status;
|
||||
}
|
||||
else
|
||||
{
|
||||
got_display_fd = (libipm_msg_in_parse(trans, "h", display_fd) == 0);
|
||||
// Close any fds we did receive to stop leaks
|
||||
if (*display_fd >= 0)
|
||||
{
|
||||
g_file_close(*display_fd);
|
||||
*display_fd = -1;
|
||||
}
|
||||
if (*chan_fd >= 0)
|
||||
{
|
||||
g_file_close(*chan_fd);
|
||||
*chan_fd = -1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Read the chansrv file descriptor
|
||||
if (rv == 0 && libipm_msg_in_parse(trans, "b", &fd_present) == 0)
|
||||
{
|
||||
if (!fd_present)
|
||||
{
|
||||
// Caller didn't send chan_fd
|
||||
*chan_fd = -1;
|
||||
}
|
||||
else
|
||||
{
|
||||
got_chan_fd = (libipm_msg_in_parse(trans, "h", chan_fd) == 0);
|
||||
}
|
||||
}
|
||||
|
||||
// If we've failed, close any file descriptors we've parsed so far
|
||||
if (rv != 0)
|
||||
{
|
||||
if (got_display_fd)
|
||||
{
|
||||
g_file_close(*display_fd);
|
||||
}
|
||||
if (got_chan_fd)
|
||||
{
|
||||
g_file_close(*chan_fd);
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
*status = (enum scp_sconnect_status)i_status;
|
||||
}
|
||||
|
||||
return rv;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user