• Ian Abbott's avatar
    staging: comedi: fix a race between do_cmd_ioctl() and read/write · 4b18f08b
    Ian Abbott authored
    `do_cmd_ioctl()` is called with the comedi device's mutex locked to
    process the `COMEDI_CMD` ioctl to set up comedi's asynchronous command
    handling on a comedi subdevice.  `comedi_read()` and `comedi_write()`
    are the `read` and `write` handlers for the comedi device, but do not
    lock the mutex (for performance reasons, as some things can hold the
    mutex for quite a long time).
    
    There is a race condition if `comedi_read()` or `comedi_write()` is
    running at the same time and for the same file object and comedi
    subdevice as `do_cmd_ioctl()`.  `do_cmd_ioctl()` sets the subdevice's
    `busy` pointer to the file object way before it sets the `SRF_RUNNING` flag
    in the subdevice's `runflags` member.  `comedi_read() and
    `comedi_write()` check the subdevice's `busy` pointer is pointing to the
    current file object, then if the `SRF_RUNNING` flag is not set, will call
    `do_become_nonbusy()` to shut down the asyncronous command.  Bad things
    can happen if the asynchronous command is being shutdown and set up at
    the same time.
    
    To prevent the race, don't set the `busy` pointer until
    after the `SRF_RUNNING` flag has been set.  Also, make sure the mutex is
    held in `comedi_read()` and `comedi_write()` while calling
    `do_become_nonbusy()` in order to avoid moving the race condition to a
    point within that function.
    
    Change some error handling `goto cleanup` statements in `do_cmd_ioctl()`
    to simple `return -ERRFOO` statements as a result of changing when the
    `busy` pointer is set.
    Signed-off-by: default avatarIan Abbott <abbotti@mev.co.uk>
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    4b18f08b
comedi_fops.c 57.2 KB