[Home]

Issue of the Implementation # L0011

Brief

drivers/hid/hidraw.c: Double mutex_lock

Detailed Description

In driver drivers/hid/hidraw.c in function hidraw_read may be double mutex_lock: Path:

line 50: begin first iteration of "while(ret==0)"

line 52: first call to mutex_lock

inner loop "while (list->head == list->tail)" does not change state of mutex, because mutex_lock immediatelly follows mutex_unlock

if we go to the second iteration of "while(ret == 0)" in line 50 then there are second call to mutex_lock in line 52 (mutex aquired twice). Second iteration of loop "while(ret==0)" is possible if local variable ret is not changed at line 94: ret+=len - i.e. len==0; Variable len may be zero if hidraw_read is called with count==0 or list->buffer[list->tail].len == 0.

  45        struct hidraw_list *list = file->private_data;
  46        int ret = 0, len;
  47        char *report;
  48        DECLARE_WAITQUEUE(wait, current);
  49
  50        while (ret == 0) {
  51
  52                mutex_lock(&list->read_mutex);
  53
  54                if (list->head == list->tail) {
  55                        add_wait_queue(&list->hidraw->wait, &wait);
  56                        set_current_state(TASK_INTERRUPTIBLE);
  57
  58                        while (list->head == list->tail) {
  59                                if (file->f_flags & O_NONBLOCK) {
  60                                        ret = -EAGAIN;
  61                                        break;
  62                                }
  63                                if (signal_pending(current)) {
  64                                        ret = -ERESTARTSYS;
  65                                        break;
  66                                }
  67                                if (!list->hidraw->exist) {
  68                                        ret = -EIO;
  69                                        break;
  70                                }
  71
  72                                /* allow O_NONBLOCK to work well from other threads */
  73                                mutex_unlock(&list->read_mutex);
  74                                schedule();
  75                                mutex_lock(&list->read_mutex);
  76                                set_current_state(TASK_INTERRUPTIBLE);
  77                        }
  78
  79                        set_current_state(TASK_RUNNING);
  80                        remove_wait_queue(&list->hidraw->wait, &wait);
  81                }
  82
  83                if (ret)
  84                        goto out;
  85
  86                report = list->buffer[list->tail].value;
  87                len = list->buffer[list->tail].len > count ?
  88                        count : list->buffer[list->tail].len;
  89
  90                if (copy_to_user(buffer, list->buffer[list->tail].value, len)) {
  91                        ret = -EFAULT;
  92                        goto out;
  93                }
  94                ret += len;
  95
  96                kfree(list->buffer[list->tail].value);
  97                list->tail = (list->tail + 1) & (HIDRAW_BUFFER_SIZE - 1);
  98        }
  99out:
 100        mutex_unlock(&list->read_mutex);
 101        return ret;

Component

linux-kernel 2.6.31

Accepted

http://lkml.org/lkml/2009/10/12/101
commit

Status

Fixed in kernel 2.6.35

[Home]