On Mon, 27 Jul 2015 12:18:33 +0300
Dan Carpenter <dan.carpen...@oracle.com> wrote:

> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
> staging-testing
> head:   59cc3399efd61fabb7f4aa23d4498bd9b01e5f6d
> commit: 9bc79bbcd0c526e3ec7b98e08c5d34648bb3c158 [413/420] Staging: most: add 
> MOST driver's aim-cdev module
> 
> drivers/staging/most/aim-cdev/cdev.c:128 aim_close() error: dereferencing 
> freed memory 'channel' 
> drivers/staging/most/aim-cdev/cdev.c:191 aim_write() error: we previously 
> assumed 'mbo' could be null (see line 170)
> 
> git remote add staging 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> git remote update staging
> git checkout 9bc79bbcd0c526e3ec7b98e08c5d34648bb3c158
> vim +/channel +128 drivers/staging/most/aim-cdev/cdev.c
> 
> 9bc79bbcd Christian Gromm 2015-07-24  122             
> atomic_dec(&channel->access_ref);
> 9bc79bbcd Christian Gromm 2015-07-24  123             
> device_destroy(aim_class, channel->devno);
> 9bc79bbcd Christian Gromm 2015-07-24  124             
> cdev_del(&channel->cdev);
> 9bc79bbcd Christian Gromm 2015-07-24  125             
> kfifo_free(&channel->fifo);
> 9bc79bbcd Christian Gromm 2015-07-24  126             
> list_del(&channel->list);
> 9bc79bbcd Christian Gromm 2015-07-24  127             kfree(channel);
> 9bc79bbcd Christian Gromm 2015-07-24 @128             
> ida_simple_remove(&minor_id, MINOR(channel->devno));
> 9bc79bbcd Christian Gromm 2015-07-24  129             
> wake_up_interruptible(&channel->wq);
> 9bc79bbcd Christian Gromm 2015-07-24  130             return 0;
> 9bc79bbcd Christian Gromm 2015-07-24  131     }
> 9bc79bbcd Christian Gromm 2015-07-24  132     
> mutex_unlock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  133  
> 9bc79bbcd Christian Gromm 2015-07-24  134     while (0 != kfifo_out((struct 
> kfifo *)&channel->fifo, &mbo, 1))
> 9bc79bbcd Christian Gromm 2015-07-24  135             most_put_mbo(mbo);
> 9bc79bbcd Christian Gromm 2015-07-24  136     if (channel->keep_mbo == true)
> 9bc79bbcd Christian Gromm 2015-07-24  137             
> most_put_mbo(channel->stacked_mbo);
> 9bc79bbcd Christian Gromm 2015-07-24  138     ret = 
> most_stop_channel(channel->iface, channel->channel_id);
> 9bc79bbcd Christian Gromm 2015-07-24  139     
> atomic_dec(&channel->access_ref);
> 9bc79bbcd Christian Gromm 2015-07-24  140     
> wake_up_interruptible(&channel->wq);
> 9bc79bbcd Christian Gromm 2015-07-24  141     return ret;
> 9bc79bbcd Christian Gromm 2015-07-24  142  }
> 9bc79bbcd Christian Gromm 2015-07-24  143  
> 9bc79bbcd Christian Gromm 2015-07-24  144  /**
> 9bc79bbcd Christian Gromm 2015-07-24  145   * aim_write - implements the 
> syscall to write to the device
> 9bc79bbcd Christian Gromm 2015-07-24  146   * @filp: file pointer
> 9bc79bbcd Christian Gromm 2015-07-24  147   * @buf: pointer to user buffer
> 9bc79bbcd Christian Gromm 2015-07-24  148   * @count: number of bytes to write
> 9bc79bbcd Christian Gromm 2015-07-24  149   * @offset: offset from where to 
> start writing
> 9bc79bbcd Christian Gromm 2015-07-24  150   */
> 9bc79bbcd Christian Gromm 2015-07-24  151  static ssize_t aim_write(struct 
> file *filp, const char __user *buf,
> 9bc79bbcd Christian Gromm 2015-07-24  152                      size_t count, 
> loff_t *offset)
> 9bc79bbcd Christian Gromm 2015-07-24  153  {
> 9bc79bbcd Christian Gromm 2015-07-24  154     int ret, err;
> 9bc79bbcd Christian Gromm 2015-07-24  155     size_t actual_len = 0;
> 9bc79bbcd Christian Gromm 2015-07-24  156     size_t max_len = 0;
> 9bc79bbcd Christian Gromm 2015-07-24  157     ssize_t retval;
> 9bc79bbcd Christian Gromm 2015-07-24  158     struct mbo *mbo;
> 9bc79bbcd Christian Gromm 2015-07-24  159     struct aim_channel *channel = 
> filp->private_data;
> 9bc79bbcd Christian Gromm 2015-07-24  160  
> 9bc79bbcd Christian Gromm 2015-07-24  161     mutex_lock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  162     if (unlikely(!channel->dev)) {
> 9bc79bbcd Christian Gromm 2015-07-24  163             
> mutex_unlock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  164             return -EPIPE;
> 9bc79bbcd Christian Gromm 2015-07-24  165     }
> 9bc79bbcd Christian Gromm 2015-07-24  166     
> mutex_unlock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  167  
> 9bc79bbcd Christian Gromm 2015-07-24  168     mbo = 
> most_get_mbo(channel->iface, channel->channel_id);
> 9bc79bbcd Christian Gromm 2015-07-24  169  
> 9bc79bbcd Christian Gromm 2015-07-24 @170     if (!mbo && channel->dev) {
> 9bc79bbcd Christian Gromm 2015-07-24  171             if ((filp->f_flags & 
> O_NONBLOCK))
> 9bc79bbcd Christian Gromm 2015-07-24  172                     return -EAGAIN;
> 9bc79bbcd Christian Gromm 2015-07-24  173             if 
> (wait_event_interruptible(
> 9bc79bbcd Christian Gromm 2015-07-24  174                         channel->wq,
> 9bc79bbcd Christian Gromm 2015-07-24  175                         (mbo = 
> most_get_mbo(channel->iface,
> 9bc79bbcd Christian Gromm 2015-07-24  176                                     
>         channel->channel_id)) ||
> 9bc79bbcd Christian Gromm 2015-07-24  177                         
> (channel->dev == NULL)))
> 9bc79bbcd Christian Gromm 2015-07-24  178                     return 
> -ERESTARTSYS;
> 9bc79bbcd Christian Gromm 2015-07-24  179     }
> 9bc79bbcd Christian Gromm 2015-07-24  180  
> 9bc79bbcd Christian Gromm 2015-07-24  181     mutex_lock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  182     if (unlikely(!channel->dev)) {
> 9bc79bbcd Christian Gromm 2015-07-24  183             
> mutex_unlock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  184             err = -EPIPE;
> 9bc79bbcd Christian Gromm 2015-07-24  185             goto error;
> 9bc79bbcd Christian Gromm 2015-07-24  186     }
> 9bc79bbcd Christian Gromm 2015-07-24  187     
> mutex_unlock(&channel->io_mutex);
> 9bc79bbcd Christian Gromm 2015-07-24  188  
> 9bc79bbcd Christian Gromm 2015-07-24  189     max_len = 
> channel->cfg->buffer_size;
> 9bc79bbcd Christian Gromm 2015-07-24  190     actual_len = min(count, 
> max_len);
> 9bc79bbcd Christian Gromm 2015-07-24 @191     mbo->buffer_length = actual_len;
Execution never gets here with mbo being null.
If "channel->dev" evaluates as true at line 170 we sleep at line 173
or else if it evaluates as false we exit the function at line 185.

But mbo needs to be checked before it is returned at line 207.

Thanks,
Chris


> 9bc79bbcd Christian Gromm 2015-07-24  192  
> 9bc79bbcd Christian Gromm 2015-07-24  193     retval = 
> copy_from_user(mbo->virt_address, buf, mbo->buffer_length);
> 9bc79bbcd Christian Gromm 2015-07-24  194     if (retval) {
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to