[lkml]   [2009]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [BUG] ati_remote2.c: possible mutex_lock without mutex_unlock
    On Tue, 13 Oct 2009, iceberg wrote:

    > In driver ./drivers/input/input.c possible call to mutex_lock from
    > function input_devices_seq_start without mutex_unlock.
    > After calling input_devices_seq_start we can't know whether
    > mutex was locked or not.

    > Case 1. If mutex_lock_interruptible was not locked due to interrupt then
    > input_devices_seq_start returns NULL.
    > Case 2. If mutex was successfuly locked but seq_list_start returned NULL
    > then input_devices_seq_start returns NULL too. The last case occurs if
    > seq_list_start is called with pos>size of input_dev_list or pos<0.
    > Hence, after calling input_devices_seq_start we can not simply check
    > that result is not NULL and call input_devices_seq_stop function
    > which unlocks the mutex. Because in case 2 the mutex will stay locked.
    > void * ret = input_devices_seq_start(...);
    > if(ret!=NULL) {
    > //mutex is acquired for sure
    > input_devices_seq_stop(...);//unlocks the mutex
    > } else {
    > //mutex may be acquired or not
    > }

    Plus, we should return EAGAIN rather than failing silently when
    input_handlers_seq_start() has been interrupted by signal, right?

    Dmitry, how about the fix below?

    From: Jiri Kosina <>
    Subject: [PATCH] Input: make input_handlers_seq_start() signal safe

    input_devices_seq_start() uses mutex_lock_interruptible() to acquire the
    input_mutex, but doesn't properly handle the situation if
    mutex_lock_interruptible() really gets interrupted. In such scenario,
    input_handlers_seq_start() returns NULL, which ambiguous, as
    seq_list_start() could return NULL as well. This could lead to the
    situation in which input_handlers_seq_stop() will try to unlock mutex that
    hasn't been locked.

    Plus, in such situations, the code fails silently, rather than returning

    Reported-by: iceberg <>
    Signed-off-by: Jiri Kosina <>
    drivers/input/input.c | 6 +++++-
    1 files changed, 5 insertions(+), 1 deletions(-)

    diff --git a/drivers/input/input.c b/drivers/input/input.c
    index c6f88eb..ef4d5c1 100644
    --- a/drivers/input/input.c
    +++ b/drivers/input/input.c
    @@ -882,7 +882,7 @@ static const struct file_operations input_devices_fileops = {
    static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos)
    if (mutex_lock_interruptible(&input_mutex))
    - return NULL;
    + return ERR_PTR(-EAGAIN);

    seq->private = (void *)(unsigned long)*pos;
    return seq_list_start(&input_handler_list, *pos);
    @@ -896,6 +896,10 @@ static void *input_handlers_seq_next(struct seq_file *seq, void *v, loff_t *pos)

    static void input_handlers_seq_stop(struct seq_file *seq, void *v)
    + /* seq_start could get interrupted by signal before acquiring mutex */
    + if (IS_ERR(v) && ERR_PTR(v) == -EAGAIN)
    + return;

    Jiri Kosina
    SUSE Labs, Novell Inc.

     \ /
      Last update: 2009-10-13 18:15    [W:0.024 / U:5.112 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site