abstrakraft / cwiid

Linux Nintendo Wiimote interface
cwiid.org
GNU General Public License v2.0
287 stars 100 forks source link

An improper locking bug on the lock wiimote->rpt_mutex #42

Open ycaibb opened 3 years ago

ycaibb commented 3 years ago

Hi, developers, thank you for your checking. It seems the lock wiimote->rpt_mutexis not released correctly when !rpt_mode & CWIID_RPT_IR==1 and exec_write_seq(wiimote, seq_len, ir_enable_seq)==1 in the function update_rpt_mode?

https://github.com/abstrakraft/cwiid/blob/fadf11e89b579bcc0336a0692ac15c93785f3f82/libcwiid/state.c#L144

int update_rpt_mode(struct wiimote *wiimote, int8_t rpt_mode)
{
    ...;

    /* rpt_mode = bitmask of requested report types */
    /* rpt_type = report id sent to the wiimote */
    if (pthread_mutex_lock(&wiimote->rpt_mutex)) {
        cwiid_err(wiimote, "Mutex lock error (rpt mutex)");
        return -1;  // <====================lack an unlock statement
    }

    ...;
    ...;
    if ((rpt_mode & CWIID_RPT_IR)) {
        if (exec_write_seq(wiimote, seq_len, ir_enable_seq)) {
            cwiid_err(wiimote, "IR enable error");
            return -1;
        }
    }

        ...;
    if (pthread_mutex_unlock(&wiimote->rpt_mutex)) {
        cwiid_err(wiimote, "Mutex unlock error (rpt mutex) - "
                  "deadlock warning");
        return -1;
    }

    return 0;
}

Best,

ycaibb commented 3 years ago

Also, the below codes could be improved to release the lock before program exit for better resource management

static CURL* get_connection(const char* path)
{
  pthread_mutex_lock(&pool_mut);
  CURL* curl = curl_pool_count ? curl_pool[--curl_pool_count] : curl_easy_init();
  if (!curl)
  {
    debugf(DBG_LEVEL_NORM, KRED"curl alloc failed");
    abort();
  }
  pthread_mutex_unlock(&pool_mut);
  return curl;
}

https://github.com/TurboGit/hubicfuse/blob/34a6c3e57467b5f7e9befe2171bf4292893c5a18/cloudfsapi.c#L102-L113