[-] Fix alsa.c null pointer deref + some minor bugs and do some housekeeping

Thanks to Denis Denisov and cppcheck for notifying about the below. The leaks
are edge cases, but the warning of dereference of avail in alsa.c points at
a bug that could probably cause actual crashes.

[src/evrtsp/rtsp.c:1352]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
[src/httpd_daap.c:228]: (error) Memory leak: s
[src/library.c:280]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[src/library.c:284]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[src/library/filescanner_playlist.c:251]: (error) Resource leak: fp
[src/library/filescanner_playlist.c:273]: (error) Resource leak: fp
[src/outputs/alsa.c:143]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
[src/outputs/alsa.c:657]: (warning) Possible null pointer dereference: avail
[src/outputs/dummy.c:75]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
[src/outputs/fifo.c:245]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
[src/outputs/raop.c:1806]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
[src/outputs/raop.c:1371]: (warning) %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'.
[src/outputs/raop.c:1471]: (warning) %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'.
[src/outputs/raop_verification.c:705] -> [src/outputs/raop_verification.c:667]: (warning) Either the condition 'if(len_M)' is redundant or there is possible null pointer dereference: len_M.
This commit is contained in:
ejurgensen 2017-10-05 22:13:01 +02:00
parent 2d27d14f3d
commit 0c2773039b
11 changed files with 41 additions and 37 deletions

View File

@ -1349,7 +1349,7 @@ evrtsp_connection_get_local_address(struct evrtsp_connection *evcon,
default:
free(*address);
address = NULL;
*address = NULL;
event_err(1, "%s: unhandled address family\n", __func__);
return;

View File

@ -211,7 +211,7 @@ daap_session_add(const char *user_agent, int request_session_id)
daap_session_cleanup();
s = (struct daap_session *)malloc(sizeof(struct daap_session));
s = calloc(1, sizeof(struct daap_session));
if (!s)
{
DPRINTF(E_LOG, L_DAAP, "Out of memory for DAAP session\n");
@ -225,6 +225,7 @@ daap_session_add(const char *user_agent, int request_session_id)
if (daap_session_get(request_session_id))
{
DPRINTF(E_LOG, L_DAAP, "Session id requested in login (%d) is not available\n", request_session_id);
free(s);
return NULL;
}
@ -385,7 +386,7 @@ daap_sort_context_new(void)
struct sort_ctx *ctx;
int ret;
ctx = (struct sort_ctx *)malloc(sizeof(struct sort_ctx));
ctx = calloc(1, sizeof(struct sort_ctx));
if (!ctx)
{
DPRINTF(E_LOG, L_DAAP, "Out of memory for sorting context\n");
@ -532,14 +533,10 @@ user_agent_filter(const char *user_agent, struct query_params *qp)
{
const char *filter;
char *buffer;
int len;
if (!user_agent)
return;
// Valgrind doesn't like strlen(filter) below, so instead we allocate 128 bytes
// to hold the string and the leading " AND ". Remember to adjust the 128 if
// you define strings here that will be too large for the buffer.
if (is_remote(user_agent))
filter = "(f.data_kind <> 1)"; // No internet radio
else
@ -547,12 +544,9 @@ user_agent_filter(const char *user_agent, struct query_params *qp)
if (qp->filter)
{
len = strlen(qp->filter) + 128;
buffer = (char *)malloc(len);
snprintf(buffer, len, "%s AND %s", qp->filter, filter);
buffer = safe_asprintf("%s AND %s", qp->filter, filter);
free(qp->filter);
qp->filter = strdup(buffer);
free(buffer);
qp->filter = buffer;
}
else
qp->filter = strdup(filter);
@ -719,7 +713,7 @@ parse_meta(struct evhttp_request *req, char *tag, const char *param, const struc
DPRINTF(E_DBG, L_DAAP, "Asking for %d meta tags\n", nmeta);
meta = (const struct dmap_field **)malloc(nmeta * sizeof(const struct dmap_field *));
meta = (const struct dmap_field **)calloc(nmeta, sizeof(const struct dmap_field *));
if (!meta)
{
DPRINTF(E_LOG, L_DAAP, "Could not allocate meta array; out of memory\n");
@ -1069,7 +1063,7 @@ daap_reply_update(struct evhttp_request *req, struct evbuffer *evbuf, char **uri
}
/* Else, just let the request hang until we have changes to push back */
ur = (struct daap_update_request *)malloc(sizeof(struct daap_update_request));
ur = calloc(1, sizeof(struct daap_update_request));
if (!ur)
{
DPRINTF(E_LOG, L_DAAP, "Out of memory for update request\n");

View File

@ -277,11 +277,11 @@ fixup_tags(struct media_file_info *mfi)
if (!mfi->album)
{
len = snprintf(NULL, 0, "%s, Season %d", mfi->tv_series_name, mfi->tv_season_num);
len = snprintf(NULL, 0, "%s, Season %u", mfi->tv_series_name, mfi->tv_season_num);
mfi->album = (char *)malloc(len + 1);
if (mfi->album)
sprintf(mfi->album, "%s, Season %d", mfi->tv_series_name, mfi->tv_season_num);
sprintf(mfi->album, "%s, Season %u", mfi->tv_series_name, mfi->tv_season_num);
}
}

View File

@ -248,7 +248,7 @@ scan_playlist(char *file, time_t mtime, int dir_id)
{
DPRINTF(E_LOG, L_SCAN, "Out of memory\n");
return;
goto out_close;
}
memset(pli, 0, sizeof(struct playlist_info));
@ -270,7 +270,7 @@ scan_playlist(char *file, time_t mtime, int dir_id)
DPRINTF(E_LOG, L_SCAN, "Error adding playlist '%s'\n", file);
free_pli(pli, 0);
return;
goto out_close;
}
DPRINTF(E_INFO, L_SCAN, "Added playlist as id %d\n", pl_id);
@ -346,11 +346,11 @@ scan_playlist(char *file, time_t mtime, int dir_id)
{
DPRINTF(E_LOG, L_SCAN, "Error reading playlist '%s': %s\n", file, strerror(errno));
fclose(fp);
return;
goto out_close;
}
fclose(fp);
DPRINTF(E_INFO, L_SCAN, "Done processing playlist\n");
out_close:
fclose(fp);
}

View File

@ -133,14 +133,15 @@ prebuf_free(struct alsa_session *as)
static void
alsa_session_free(struct alsa_session *as)
{
if (!as)
return;
event_free(as->deferredev);
prebuf_free(as);
free(as->output_session);
free(as);
as = NULL;
}
static void
@ -654,6 +655,7 @@ buffer_write(struct alsa_session *as, uint8_t *buf, snd_pcm_sframes_t *avail, in
if (ret != nsamp)
DPRINTF(E_WARN, L_LAUDIO, "ALSA partial write detected\n");
if (avail)
*avail -= ret;
return 0;

View File

@ -509,6 +509,9 @@ squote_to_dquote(char *buf)
static void
cast_session_free(struct cast_session *cs)
{
if (!cs)
return;
event_free(cs->reply_timeout);
event_free(cs->ev);

View File

@ -67,12 +67,13 @@ defer_cb(int fd, short what, void *arg);
static void
dummy_session_free(struct dummy_session *ds)
{
if (!ds)
return;
event_free(ds->deferredev);
free(ds->output_session);
free(ds);
ds = NULL;
}
static void

View File

@ -237,12 +237,14 @@ fifo_empty(struct fifo_session *fifo_session)
static void
fifo_session_free(struct fifo_session *fifo_session)
{
if (!fifo_session)
return;
event_free(fifo_session->deferredev);
free(fifo_session->output_session);
free(fifo_session);
free_buffer();
fifo_session = NULL;
}
static void

View File

@ -97,6 +97,9 @@ pulse_from_device_volume(int device_volume)
static void
pulse_session_free(struct pulse_session *ps)
{
if (!ps)
return;
if (ps->stream)
{
pa_threaded_mainloop_lock(pulse.mainloop);

View File

@ -1368,7 +1368,7 @@ raop_send_req_flush(struct raop_session *rs, uint64_t rtptime, evrtsp_req_cb cb)
}
/* Restart sequence: last sequence + 1 */
ret = snprintf(buf, sizeof(buf), "seq=%u;rtptime=%u", stream_seq + 1, RAOP_RTPTIME(rtptime));
ret = snprintf(buf, sizeof(buf), "seq=%" PRIu16 ";rtptime=%u", stream_seq + 1, RAOP_RTPTIME(rtptime));
if ((ret < 0) || (ret >= sizeof(buf)))
{
DPRINTF(E_LOG, L_RAOP, "RTP-Info too big for buffer in FLUSH request\n");
@ -1468,7 +1468,7 @@ raop_send_req_record(struct raop_session *rs, evrtsp_req_cb cb)
evrtsp_add_header(req->output_headers, "Range", "npt=0-");
/* Start sequence: next sequence */
ret = snprintf(buf, sizeof(buf), "seq=%u;rtptime=%u", stream_seq + 1, RAOP_RTPTIME(rs->start_rtptime));
ret = snprintf(buf, sizeof(buf), "seq=%" PRIu16 ";rtptime=%u", stream_seq + 1, RAOP_RTPTIME(rs->start_rtptime));
if ((ret < 0) || (ret >= sizeof(buf)))
{
DPRINTF(E_LOG, L_RAOP, "RTP-Info too big for buffer in RECORD request\n");
@ -1778,6 +1778,9 @@ raop_status(struct raop_session *rs)
static void
raop_session_free(struct raop_session *rs)
{
if (!rs)
return;
evrtsp_connection_set_closecb(rs->ctrl, NULL, NULL);
evrtsp_connection_free(rs->ctrl);
@ -1802,8 +1805,6 @@ raop_session_free(struct raop_session *rs)
free(rs->output_session);
free(rs);
rs = NULL;
}
static void
@ -3461,7 +3462,7 @@ raop_v2_resend_range(struct raop_session *rs, uint16_t seqnum, uint16_t len)
/* Check that seqnum is in the retransmit buffer */
if ((seqnum > pktbuf_head->seqnum) || (seqnum < pktbuf_tail->seqnum))
{
DPRINTF(E_WARN, L_RAOP, "Device '%s' asking for seqnum %u; not in buffer (h %u t %u)\n", rs->devname, seqnum, pktbuf_head->seqnum, pktbuf_tail->seqnum);
DPRINTF(E_WARN, L_RAOP, "Device '%s' asking for seqnum %" PRIu16 "; not in buffer (h %" PRIu16 " t %" PRIu16 ")\n", rs->devname, seqnum, pktbuf_head->seqnum, pktbuf_tail->seqnum);
return;
}

View File

@ -702,13 +702,11 @@ srp_user_process_challenge(struct SRPUser *usr, const unsigned char *bytes_s, in
calculate_H_AMK(usr->alg, usr->H_AMK, usr->A, usr->M, usr->session_key, usr->session_key_len);
*bytes_M = usr->M;
if (len_M)
*len_M = hash_length(usr->alg);
}
else
{
*bytes_M = NULL;
if (len_M)
*len_M = 0;
}