From e63a01249d9ce867e7c91abf8c51e5c29ec81f0a Mon Sep 17 00:00:00 2001 From: Marcin Chrzanowski Date: Mon, 28 May 2018 18:49:34 +0200 Subject: Refactor and better error handling --- char.c | 339 ++-------------------------------------------------------- pci.c | 4 +- surface.c | 358 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- surface.h | 3 + 4 files changed, 364 insertions(+), 340 deletions(-) diff --git a/char.c b/char.c index bb70d1b..ae718c6 100644 --- a/char.c +++ b/char.c @@ -19,343 +19,24 @@ dev_t first; int major; int next_minor = 0; -long doom_create_surface(struct file *filp, unsigned long arg) -{ - struct doomdev_ioctl_create_surface *params; - - params = (struct doomdev_ioctl_create_surface *)arg; - return new_surface(filp, params); -} - -void free_texture(struct texture_data *texture_data); -int texture_release(struct inode *inode, struct file *filp) -{ - struct texture_data *texture_data; - - texture_data = filp->private_data; - - free_texture(texture_data); - - kfree(texture_data); - - return 0; -} - -struct file_operations texture_fops = { - .owner = THIS_MODULE, - .release = texture_release - -}; - -int verify_texture_params(struct doomdev_ioctl_create_texture *params) -{ - if (params->size > 4 * 1024 * 1024) { - return -EOVERFLOW; - } - - if (params->height > 1023) { - return -EOVERFLOW; - } - - return 0; -} - -int alloc_texture(struct doomdev_ioctl_create_texture *params, - struct texture_data *texture_data) -{ - int err; - int i; - int not_written; - int pages_needed; - - texture_data->size = params->size; - texture_data->height = params->height; - - pages_needed = (params->size / HARDDOOM_PAGE_SIZE); - if (params->size % HARDDOOM_PAGE_SIZE != 0) { - pages_needed += 1; - } - - texture_data->pages = pages_needed; - texture_data->texture_cpu = - dma_alloc_coherent(texture_data->doom_data->pci_device, - params->size, &texture_data->texture_dev, - GFP_KERNEL); - ORFAIL_NULL(texture_data->texture_cpu, -ENOMEM, error_texture); - - texture_data->page_table_cpu = - dma_alloc_coherent(texture_data->doom_data->pci_device, - pages_needed * 4, &texture_data->page_table_dev, - GFP_KERNEL); - ORFAIL_NULL(texture_data->page_table_cpu, -ENOMEM, error_pt); - - for (i = 0; i < pages_needed; i++) { - texture_data->page_table_cpu[i] = - (HARDDOOM_PTE_PHYS_MASK & - (texture_data->texture_dev + HARDDOOM_PAGE_SIZE * i)) | - HARDDOOM_PTE_VALID; - } - - not_written = copy_from_user(texture_data->texture_cpu, - (void __user *) params->data_ptr, - params->size); - - if (not_written) { - err = -EFAULT; - goto error_copy; - } - - return 0; - -error_copy: - dma_free_coherent(texture_data->doom_data->pci_device, - texture_data->pages * 4, texture_data->page_table_cpu, - texture_data->page_table_dev); -error_pt: - dma_free_coherent(texture_data->doom_data->pci_device, - texture_data->size, texture_data->texture_cpu, - texture_data->texture_dev); -error_texture: - return err; -} - -void free_texture(struct texture_data *texture_data) -{ - dma_free_coherent(texture_data->doom_data->pci_device, - texture_data->pages * 4, texture_data->page_table_cpu, - texture_data->page_table_dev); - - dma_free_coherent(texture_data->doom_data->pci_device, - texture_data->size, texture_data->texture_cpu, - texture_data->texture_dev); -} - -long doom_create_texture(struct file *filp, unsigned long arg) -{ - int err; - struct doomdev_ioctl_create_texture *params; - struct texture_data *texture_data; - int fd; - struct doom_data *doom_data; - - params = (struct doomdev_ioctl_create_texture *) arg; - - err = verify_texture_params(params); - if (err < 0) { - return err; - } - - texture_data = kmalloc(sizeof(*texture_data), GFP_KERNEL); - ORFAIL_NULL(texture_data, -ENOMEM, error_data); - doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); - texture_data->doom_data = doom_data; - - ORFAIL(alloc_texture(params, texture_data), error_texture); - - fd = anon_inode_getfd("doom_texture", &texture_fops, texture_data, 0); - ORFAIL(fd, error_inode); - - return fd; - -error_inode: - free_texture(texture_data); -error_texture: - kfree(texture_data); -error_data: - return err; -} - -void free_flat(struct flat_data *flat_data); -int flat_release(struct inode *inode, struct file *filp) -{ - struct flat_data *flat_data; - - flat_data = filp->private_data; - - free_flat(flat_data); - - kfree(flat_data); - - return 0; -} - -struct file_operations flat_fops = { - .owner = THIS_MODULE, - .release = flat_release - -}; - -int alloc_flat(struct doomdev_ioctl_create_flat *params, - struct flat_data *flat_data) -{ - int err; - int not_written; - - flat_data->flat_cpu = - dma_alloc_coherent(flat_data->doom_data->pci_device, - HARDDOOM_FLAT_SIZE, &flat_data->flat_dev, - GFP_KERNEL); - ORFAIL_NULL(flat_data->flat_cpu, -ENOMEM, error_flat); - - not_written = copy_from_user(flat_data->flat_cpu, - (void __user *) params->data_ptr, HARDDOOM_FLAT_SIZE); - - if (not_written) { - err = -EFAULT; - goto error_copy; - } - - return 0; - -error_copy: - dma_free_coherent(flat_data->doom_data->pci_device, HARDDOOM_FLAT_SIZE, - flat_data->flat_cpu, flat_data->flat_dev); -error_flat: - return err; - -} - -void free_flat(struct flat_data *flat_data) -{ - dma_free_coherent(flat_data->doom_data->pci_device, HARDDOOM_FLAT_SIZE, - flat_data->flat_cpu, flat_data->flat_dev); -} - -long doom_create_flat(struct file *filp, unsigned long arg) -{ - int err; - struct doomdev_ioctl_create_flat *params; - struct flat_data *flat_data; - int fd; - struct doom_data *doom_data; - - params = (struct doomdev_ioctl_create_flat *) arg; - - flat_data = kmalloc(sizeof(*flat_data), GFP_KERNEL); - ORFAIL_NULL(flat_data, -ENOMEM, error_data); - doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); - flat_data->doom_data = doom_data; - - ORFAIL(alloc_flat(params, flat_data), error_flat); - - fd = anon_inode_getfd("doom_flat", &flat_fops, flat_data, 0); - ORFAIL(fd, error_inode); - - return fd; - -error_inode: - free_flat(flat_data); -error_flat: - kfree(flat_data); -error_data: - return err; -} - -void free_colors(struct colors_data *colors_data); -int colors_release(struct inode *inode, struct file *filp) -{ - struct colors_data *colors_data; - - colors_data = filp->private_data; - - free_colors(colors_data); - - kfree(colors_data); - - return 0; -} - -struct file_operations colors_fops = { - .owner = THIS_MODULE, - .release = colors_release - -}; - -int alloc_colors(struct doomdev_ioctl_create_colormaps *params, - struct colors_data *colors_data) -{ - int err; - int not_written; - - colors_data->number = params->num; - - colors_data->colors_cpu = - dma_alloc_coherent(colors_data->doom_data->pci_device, - HARDDOOM_COLORMAP_SIZE * params->num, - &colors_data->colors_dev, - GFP_KERNEL); - ORFAIL_NULL(colors_data->colors_cpu, -ENOMEM, error_colors); - - not_written = copy_from_user(colors_data->colors_cpu, - (void __user *) params->data_ptr, - HARDDOOM_COLORMAP_SIZE * params->num); - - if (not_written) { - err = -EFAULT; - goto error_copy; - } - - return 0; - -error_copy: - dma_free_coherent(colors_data->doom_data->pci_device, - HARDDOOM_COLORMAP_SIZE * params->num, - colors_data->colors_cpu, colors_data->colors_dev); -error_colors: - return err; - -} - -void free_colors(struct colors_data *colors_data) -{ - dma_free_coherent(colors_data->doom_data->pci_device, - HARDDOOM_COLORMAP_SIZE * colors_data->number, - colors_data->colors_cpu, colors_data->colors_dev); -} - -long doom_create_colormaps(struct file *filp, unsigned long arg) -{ - int err; - struct doomdev_ioctl_create_colormaps *params; - struct colors_data *colors_data; - int fd; - struct doom_data *doom_data; - - params = (struct doomdev_ioctl_create_colormaps *) arg; - - colors_data = kmalloc(sizeof(*colors_data), GFP_KERNEL); - ORFAIL_NULL(colors_data, -ENOMEM, error_data); - doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); - colors_data->doom_data = doom_data; - - ORFAIL(alloc_colors(params, colors_data), error_colors); - - fd = anon_inode_getfd("doom_colors", &colors_fops, colors_data, 0); - ORFAIL(fd, error_inode); - - return fd; - -error_inode: - free_colors(colors_data); -error_colors: - kfree(colors_data); -error_data: - return err; -} long doom_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { switch (cmd) { case DOOMDEV_IOCTL_CREATE_SURFACE: - return doom_create_surface(filp, arg); + return new_surface(filp, + (struct doomdev_ioctl_create_surface *) arg); case DOOMDEV_IOCTL_CREATE_TEXTURE: - return doom_create_texture(filp, arg); + return new_texture(filp, + (struct doomdev_ioctl_create_texture *) arg); case DOOMDEV_IOCTL_CREATE_FLAT: - return doom_create_flat(filp, arg); + return new_flat(filp, + (struct doomdev_ioctl_create_flat *) arg); case DOOMDEV_IOCTL_CREATE_COLORMAPS: - return doom_create_colormaps(filp, arg); + return new_colors(filp, + (struct doomdev_ioctl_create_colormaps *) arg); default: - return -EINVAL; + return -ENOTTY; } } @@ -381,7 +62,7 @@ int new_doomdev(struct pci_dev *dev) dev_t devt; if (next_minor >= DOOMDEV_COUNT) { - return -ENOMEM; + return -EOVERFLOW; } doom_data = pci_get_drvdata(dev); diff --git a/pci.c b/pci.c index 69783d6..11f6256 100644 --- a/pci.c +++ b/pci.c @@ -33,7 +33,7 @@ irqreturn_t doom_irq(int irq, void *dev) interrupts = ioread32(iomem + HARDDOOM_INTR) & ioread32(iomem + HARDDOOM_INTR_ENABLE); iowrite32(interrupts, iomem + HARDDOOM_INTR); - + if (!interrupts) { return IRQ_NONE; } @@ -65,6 +65,7 @@ int init_pci(struct pci_dev *dev) } pci_set_drvdata(dev, doom_data); doom_data->iomem = pci_iomap(dev, 0, 0); + ORFAIL_NULL(doom_data->iomem, -ENOMEM, error_iomem); doom_data->pci_device = &dev->dev; pci_set_master(dev); @@ -85,6 +86,7 @@ int init_pci(struct pci_dev *dev) error_irq: pci_clear_master(dev); pci_iounmap(dev, doom_data->iomem); +error_iomem: kfree(doom_data); error_kmalloc: pci_release_regions(dev); diff --git a/surface.c b/surface.c index 13b0e6b..35c68bc 100644 --- a/surface.c +++ b/surface.c @@ -11,6 +11,322 @@ #include "util.h" #include "harddoomdev.h" +void free_texture(struct texture_data *texture_data); +int texture_release(struct inode *inode, struct file *filp) +{ + struct texture_data *texture_data; + + texture_data = filp->private_data; + + free_texture(texture_data); + + kfree(texture_data); + + return 0; +} + +struct file_operations texture_fops = { + .owner = THIS_MODULE, + .release = texture_release + +}; + +int verify_texture_params(struct doomdev_ioctl_create_texture *params) +{ + if (params->size > 4 * 1024 * 1024) { + return -EOVERFLOW; + } + + if (params->height > 1023) { + return -EOVERFLOW; + } + + return 0; +} + +int alloc_texture(struct doomdev_ioctl_create_texture *params, + struct texture_data *texture_data) +{ + int err; + int i; + int not_written; + int pages_needed; + + texture_data->size = params->size; + texture_data->height = params->height; + + pages_needed = (params->size / HARDDOOM_PAGE_SIZE); + if (params->size % HARDDOOM_PAGE_SIZE != 0) { + pages_needed += 1; + } + + texture_data->pages = pages_needed; + texture_data->texture_cpu = + dma_alloc_coherent(texture_data->doom_data->pci_device, + params->size, &texture_data->texture_dev, + GFP_KERNEL); + ORFAIL_NULL(texture_data->texture_cpu, -ENOMEM, error_texture); + + texture_data->page_table_cpu = + dma_alloc_coherent(texture_data->doom_data->pci_device, + pages_needed * 4, &texture_data->page_table_dev, + GFP_KERNEL); + ORFAIL_NULL(texture_data->page_table_cpu, -ENOMEM, error_pt); + + for (i = 0; i < pages_needed; i++) { + texture_data->page_table_cpu[i] = + (HARDDOOM_PTE_PHYS_MASK & + (texture_data->texture_dev + HARDDOOM_PAGE_SIZE * i)) | + HARDDOOM_PTE_VALID; + } + + not_written = copy_from_user(texture_data->texture_cpu, + (void __user *) params->data_ptr, + params->size); + + if (not_written) { + err = -EFAULT; + goto error_copy; + } + + return 0; + +error_copy: + dma_free_coherent(texture_data->doom_data->pci_device, + texture_data->pages * 4, texture_data->page_table_cpu, + texture_data->page_table_dev); +error_pt: + dma_free_coherent(texture_data->doom_data->pci_device, + texture_data->size, texture_data->texture_cpu, + texture_data->texture_dev); +error_texture: + return err; +} + +void free_texture(struct texture_data *texture_data) +{ + dma_free_coherent(texture_data->doom_data->pci_device, + texture_data->pages * 4, texture_data->page_table_cpu, + texture_data->page_table_dev); + + dma_free_coherent(texture_data->doom_data->pci_device, + texture_data->size, texture_data->texture_cpu, + texture_data->texture_dev); +} + +int new_texture(struct file *filp, struct doomdev_ioctl_create_texture *arg) +{ + int err; + struct doomdev_ioctl_create_texture *params; + struct texture_data *texture_data; + int fd; + struct doom_data *doom_data; + + params = (struct doomdev_ioctl_create_texture *) arg; + + err = verify_texture_params(params); + if (err < 0) { + return err; + } + + texture_data = kmalloc(sizeof(*texture_data), GFP_KERNEL); + ORFAIL_NULL(texture_data, -ENOMEM, error_data); + doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); + texture_data->doom_data = doom_data; + + ORFAIL(alloc_texture(params, texture_data), error_texture); + + fd = anon_inode_getfd("doom_texture", &texture_fops, texture_data, 0); + ORFAIL(fd, error_inode); + + return fd; + +error_inode: + free_texture(texture_data); +error_texture: + kfree(texture_data); +error_data: + return err; +} + +void free_flat(struct flat_data *flat_data); +int flat_release(struct inode *inode, struct file *filp) +{ + struct flat_data *flat_data; + + flat_data = filp->private_data; + + free_flat(flat_data); + + kfree(flat_data); + + return 0; +} + +struct file_operations flat_fops = { + .owner = THIS_MODULE, + .release = flat_release + +}; + +int alloc_flat(struct doomdev_ioctl_create_flat *params, + struct flat_data *flat_data) +{ + int err; + int not_written; + + flat_data->flat_cpu = + dma_alloc_coherent(flat_data->doom_data->pci_device, + HARDDOOM_FLAT_SIZE, &flat_data->flat_dev, + GFP_KERNEL); + ORFAIL_NULL(flat_data->flat_cpu, -ENOMEM, error_flat); + + not_written = copy_from_user(flat_data->flat_cpu, + (void __user *) params->data_ptr, HARDDOOM_FLAT_SIZE); + + if (not_written) { + err = -EFAULT; + goto error_copy; + } + + return 0; + +error_copy: + dma_free_coherent(flat_data->doom_data->pci_device, HARDDOOM_FLAT_SIZE, + flat_data->flat_cpu, flat_data->flat_dev); +error_flat: + return err; + +} + +void free_flat(struct flat_data *flat_data) +{ + dma_free_coherent(flat_data->doom_data->pci_device, HARDDOOM_FLAT_SIZE, + flat_data->flat_cpu, flat_data->flat_dev); +} + +int new_flat(struct file *filp, struct doomdev_ioctl_create_flat *arg) +{ + int err; + struct doomdev_ioctl_create_flat *params; + struct flat_data *flat_data; + int fd; + struct doom_data *doom_data; + + params = (struct doomdev_ioctl_create_flat *) arg; + + flat_data = kmalloc(sizeof(*flat_data), GFP_KERNEL); + ORFAIL_NULL(flat_data, -ENOMEM, error_data); + doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); + flat_data->doom_data = doom_data; + + ORFAIL(alloc_flat(params, flat_data), error_flat); + + fd = anon_inode_getfd("doom_flat", &flat_fops, flat_data, 0); + ORFAIL(fd, error_inode); + + return fd; + +error_inode: + free_flat(flat_data); +error_flat: + kfree(flat_data); +error_data: + return err; +} + +void free_colors(struct colors_data *colors_data); +int colors_release(struct inode *inode, struct file *filp) +{ + struct colors_data *colors_data; + + colors_data = filp->private_data; + + free_colors(colors_data); + + kfree(colors_data); + + return 0; +} + +struct file_operations colors_fops = { + .owner = THIS_MODULE, + .release = colors_release + +}; + +int alloc_colors(struct doomdev_ioctl_create_colormaps *params, + struct colors_data *colors_data) +{ + int err; + int not_written; + + colors_data->number = params->num; + + colors_data->colors_cpu = + dma_alloc_coherent(colors_data->doom_data->pci_device, + HARDDOOM_COLORMAP_SIZE * params->num, + &colors_data->colors_dev, + GFP_KERNEL); + ORFAIL_NULL(colors_data->colors_cpu, -ENOMEM, error_colors); + + not_written = copy_from_user(colors_data->colors_cpu, + (void __user *) params->data_ptr, + HARDDOOM_COLORMAP_SIZE * params->num); + + if (not_written) { + err = -EFAULT; + goto error_copy; + } + + return 0; + +error_copy: + dma_free_coherent(colors_data->doom_data->pci_device, + HARDDOOM_COLORMAP_SIZE * params->num, + colors_data->colors_cpu, colors_data->colors_dev); +error_colors: + return err; + +} + +void free_colors(struct colors_data *colors_data) +{ + dma_free_coherent(colors_data->doom_data->pci_device, + HARDDOOM_COLORMAP_SIZE * colors_data->number, + colors_data->colors_cpu, colors_data->colors_dev); +} + +int new_colors(struct file *filp, struct doomdev_ioctl_create_colormaps *arg) +{ + int err; + struct doomdev_ioctl_create_colormaps *params; + struct colors_data *colors_data; + int fd; + struct doom_data *doom_data; + + params = (struct doomdev_ioctl_create_colormaps *) arg; + + colors_data = kmalloc(sizeof(*colors_data), GFP_KERNEL); + ORFAIL_NULL(colors_data, -ENOMEM, error_data); + doom_data = container_of(filp->f_inode->i_cdev, struct doom_data, cdev); + colors_data->doom_data = doom_data; + + ORFAIL(alloc_colors(params, colors_data), error_colors); + + fd = anon_inode_getfd("doom_colors", &colors_fops, colors_data, 0); + ORFAIL(fd, error_inode); + + return fd; + +error_inode: + free_colors(colors_data); +error_colors: + kfree(colors_data); +error_data: + return err; +} + long draw_lines(struct file *filp, unsigned long arg) { struct surface_data *surface_data; @@ -102,7 +418,9 @@ long draw_columns(struct file *filp, unsigned long arg) uint8_t flags; bool got_colors = false; bool got_trans = false; + bool got_texture = false; int i; + int err; surface_data = filp->private_data; @@ -117,31 +435,45 @@ long draw_columns(struct file *filp, unsigned long arg) } colors_fds = fdget(param->colormaps_fd); - colors_data = colors_fds.file->private_data; - got_colors = true; + if (colors_fds.file->f_op != &colors_fops) { + err = -EINVAL; + goto error_fdget; + } + colors_data = colors_fds.file->private_data; if (surface_data->doom_data != colors_data->doom_data) { - return -EINVAL; + err = -EINVAL; + goto error_fdget; } } if (flags & DOOMDEV_DRAW_FLAGS_TRANSLATE) { trans_fds = fdget(param->translations_fd); - trans_data = trans_fds.file->private_data; - got_trans = true; + if (trans_fds.file->f_op != &colors_fops) { + err = -EINVAL; + goto error_fdget; + } + trans_data = trans_fds.file->private_data; if (surface_data->doom_data != trans_data->doom_data) { - return -EINVAL; + err = -EINVAL; + goto error_fdget; } } texture_fds = fdget(param->texture_fd); - texture_data = texture_fds.file->private_data; + got_texture = true; + if (texture_fds.file->f_op != &texture_fops) { + err = -EINVAL; + goto error_fdget; + } + texture_data = texture_fds.file->private_data; if (surface_data->doom_data != texture_data->doom_data) { - return -EINVAL; + err = -EINVAL; + goto error_fdget; } columns = (struct doomdev_column *) param->columns_ptr; @@ -153,9 +485,15 @@ long draw_columns(struct file *filp, unsigned long arg) trans_data, flags, param->translation_idx); } + err = param->columns_num; + mutex_unlock(&surface_data->doom_data->cmd_mutex); - fdput(texture_fds); +error_fdget: + + if (got_texture) { + fdput(texture_fds); + } if (got_colors) { fdput(colors_fds); @@ -248,7 +586,7 @@ long surface_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) case DOOMDEV_SURF_IOCTL_DRAW_BACKGROUND: return do_draw_background(filp, arg); default: - return -1; + return -ENOTTY; } } diff --git a/surface.h b/surface.h index bd65b66..70016a1 100644 --- a/surface.h +++ b/surface.h @@ -4,5 +4,8 @@ #include "doomdev.h" int new_surface(struct file *filp, struct doomdev_ioctl_create_surface *params); +int new_colors(struct file *filp, struct doomdev_ioctl_create_colormaps *params); +int new_texture(struct file *filp, struct doomdev_ioctl_create_texture *params); +int new_flat(struct file *filp, struct doomdev_ioctl_create_flat *params); #endif -- cgit v1.2.3