From 22ec8ebb17d959486e4a865b17115e609eb688ee Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 26 Aug 2005 23:27:19 +0000 Subject: - Don't try to allocate mappings of less than a PAGE_SIZE in MGA DMA code. - Comment out the "is this mapping/bufs in allocated AGP" bits in BSD because they break mga (which uses AGP allocation that doesn't track entries). It's not a security issue when we still have the related ioctls marked root-only. - Apply some power-of-two alignment restrictions to hopefully avoid some panicing in bad cases of drm_pci_alloc() on FreeBSD. - Add verbosity to some error handling that I found useful while debugging. --- bsd-core/drm_bufs.c | 73 +++++++++++++++++++++++++++++++++------------------ bsd-core/drm_pci.c | 7 +++++ shared-core/mga_dma.c | 51 +++++++++++++++++++++-------------- 3 files changed, 86 insertions(+), 45 deletions(-) diff --git a/bsd-core/drm_bufs.c b/bsd-core/drm_bufs.c index 1df0238d..0e3fc1fd 100644 --- a/bsd-core/drm_bufs.c +++ b/bsd-core/drm_bufs.c @@ -96,17 +96,28 @@ int drm_addmap(drm_device_t * dev, unsigned long offset, unsigned long size, drm_map_type_t type, drm_map_flags_t flags, drm_local_map_t **map_ptr) { drm_local_map_t *map; + int align; + /*drm_agp_mem_t *entry; + int valid;*/ /* Only allow shared memory to be removable since we only keep enough * book keeping information about shared memory to allow for removal * when processes fork. */ - if ((flags & _DRM_REMOVABLE) && type != _DRM_SHM) + if ((flags & _DRM_REMOVABLE) && type != _DRM_SHM) { + DRM_ERROR("Requested removable map for non-DRM_SHM\n"); return EINVAL; - if ((offset & PAGE_MASK) || (size & PAGE_MASK)) + } + if ((offset & PAGE_MASK) || (size & PAGE_MASK)) { + DRM_ERROR("offset/size not page aligned: 0x%lx/0x%lx\n", + offset, size); return EINVAL; - if (offset + size < offset) + } + if (offset + size < offset) { + DRM_ERROR("offset and size wrap around: 0x%lx/0x%lx\n", + offset, size); return EINVAL; + } DRM_DEBUG("offset = 0x%08lx, size = 0x%08lx, type = %d\n", offset, size, type); @@ -173,25 +184,21 @@ int drm_addmap(drm_device_t * dev, unsigned long offset, unsigned long size, } break; case _DRM_AGP: - { - drm_agp_mem_t *entry; - int valid = 0; - - map->offset += dev->agp->base; - map->mtrr = dev->agp->mtrr; /* for getmap */ - for (entry = dev->agp->memory; entry; - entry = entry->next) - if ((map->offset >= entry->bound) && - (map->offset + map->size <= - entry->bound + entry->pages * PAGE_SIZE)) { - valid = 1; - break; - } - if (!valid) { - free(map, M_DRM); - return DRM_ERR(EACCES); + /*valid = 0;*/ + map->offset += dev->agp->base; + map->mtrr = dev->agp->mtrr; /* for getmap */ + /*for (entry = dev->agp->memory; entry; entry = entry->next) { + if ((map->offset >= entry->bound) && + (map->offset + map->size <= + entry->bound + entry->pages * PAGE_SIZE)) { + valid = 1; + break; } } + if (!valid) { + free(map, M_DRM); + return DRM_ERR(EACCES); + }*/ break; case _DRM_SCATTER_GATHER: if (!dev->sg) { @@ -201,8 +208,16 @@ int drm_addmap(drm_device_t * dev, unsigned long offset, unsigned long size, map->offset = map->offset + dev->sg->handle; break; case _DRM_CONSISTENT: - map->dmah = drm_pci_alloc(dev, map->size, map->size, - 0xfffffffful); + /* Unfortunately, we don't get any alignment specification from + * the caller, so we have to guess. drm_pci_alloc requires + * a power-of-two alignment, so try to align the bus address of + * the map to it size if possible, otherwise just assume + * PAGE_SIZE alignment. + */ + align = map->size; + if ((align & (align - 1)) != 0) + align = PAGE_SIZE; + map->dmah = drm_pci_alloc(dev, map->size, align, 0xfffffffful); if (map->dmah == NULL) { free(map, M_DRM); return DRM_ERR(ENOMEM); @@ -211,6 +226,7 @@ int drm_addmap(drm_device_t * dev, unsigned long offset, unsigned long size, map->offset = map->dmah->busaddr; break; default: + DRM_ERROR("Bad map type %d\n", map->type); free(map, M_DRM); return DRM_ERR(EINVAL); } @@ -364,7 +380,8 @@ static int drm_do_addbufs_agp(drm_device_t *dev, drm_buf_desc_t *request) { drm_device_dma_t *dma = dev->dma; drm_buf_entry_t *entry; - drm_agp_mem_t *agp_entry; + /*drm_agp_mem_t *agp_entry; + int valid*/ drm_buf_t *buf; unsigned long offset; unsigned long agp_offset; @@ -375,7 +392,7 @@ static int drm_do_addbufs_agp(drm_device_t *dev, drm_buf_desc_t *request) int page_order; int total; int byte_count; - int i, valid; + int i; drm_buf_t **temp_buflist; count = request->count; @@ -399,7 +416,11 @@ static int drm_do_addbufs_agp(drm_device_t *dev, drm_buf_desc_t *request) DRM_DEBUG( "total: %d\n", total ); /* Make sure buffers are located in AGP memory that we own */ - valid = 0; + /* Breaks MGA due to drm_alloc_agp not setting up entries for the + * memory. Safe to ignore for now because these ioctls are still + * root-only. + */ + /*valid = 0; for (agp_entry = dev->agp->memory; agp_entry; agp_entry = agp_entry->next) { if ((agp_offset >= agp_entry->bound) && @@ -412,7 +433,7 @@ static int drm_do_addbufs_agp(drm_device_t *dev, drm_buf_desc_t *request) if (!valid) { DRM_DEBUG("zone invalid\n"); return DRM_ERR(EINVAL); - } + }*/ entry = &dma->bufs[order]; diff --git a/bsd-core/drm_pci.c b/bsd-core/drm_pci.c index 0f431dd6..a33f5f9c 100644 --- a/bsd-core/drm_pci.c +++ b/bsd-core/drm_pci.c @@ -58,6 +58,13 @@ drm_pci_alloc(drm_device_t *dev, size_t size, size_t align, dma_addr_t maxaddr) drm_dma_handle_t *dmah; int ret; + /* Need power-of-two alignment, so fail the allocation if it isn't. */ + if ((align & (align - 1)) != 0) { + DRM_ERROR("drm_pci_alloc with non-power-of-two alignment %d\n", + (int)align); + return NULL; + } + dmah = malloc(sizeof(drm_dma_handle_t), M_DRM, M_ZERO | M_NOWAIT); if (dmah == NULL) return NULL; diff --git a/shared-core/mga_dma.c b/shared-core/mga_dma.c index 005c5234..c61ba1aa 100644 --- a/shared-core/mga_dma.c +++ b/shared-core/mga_dma.c @@ -435,7 +435,7 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, drm_mga_dma_bootstrap_t * dma_bs) { drm_mga_private_t * const dev_priv = (drm_mga_private_t *) dev->dev_private; - const unsigned int warp_size = mga_warp_microcode_size(dev_priv); + unsigned int warp_size = mga_warp_microcode_size(dev_priv); int err; unsigned offset; const unsigned secondary_size = dma_bs->secondary_bin_count @@ -448,13 +448,13 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, /* Acquire AGP. */ err = drm_agp_acquire(dev); if (err) { - DRM_ERROR("Unable to acquire AGP\n"); + DRM_ERROR("Unable to acquire AGP: %d\n", err); return err; } err = drm_agp_info(dev, &info); if (err) { - DRM_ERROR("Unable to get AGP info\n"); + DRM_ERROR("Unable to get AGP info: %d\n", err); return err; } @@ -490,15 +490,21 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, err = drm_bind_agp( dev_priv->agp_mem, 0 ); if (err) { - DRM_ERROR("Unable to bind AGP memory\n"); + DRM_ERROR("Unable to bind AGP memory: %d\n", err); return err; } + /* Make drm_addbufs happy by not trying to create a mapping for less + * than a page. + */ + if (warp_size < PAGE_SIZE) + warp_size = PAGE_SIZE; + offset = 0; err = drm_addmap( dev, offset, warp_size, _DRM_AGP, _DRM_READ_ONLY, & dev_priv->warp ); if (err) { - DRM_ERROR("Unable to map WARP microcode\n"); + DRM_ERROR("Unable to map WARP microcode: %d\n", err); return err; } @@ -506,7 +512,7 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, err = drm_addmap( dev, offset, dma_bs->primary_size, _DRM_AGP, _DRM_READ_ONLY, & dev_priv->primary ); if (err) { - DRM_ERROR("Unable to map primary DMA region\n"); + DRM_ERROR("Unable to map primary DMA region: %d\n", err); return err; } @@ -514,7 +520,7 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, err = drm_addmap( dev, offset, secondary_size, _DRM_AGP, 0, & dev->agp_buffer_map ); if (err) { - DRM_ERROR("Unable to map secondary DMA region\n"); + DRM_ERROR("Unable to map secondary DMA region: %d\n", err); return err; } @@ -526,7 +532,7 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, err = drm_addbufs_agp( dev, & req ); if (err) { - DRM_ERROR("Unable to add secondary DMA buffers\n"); + DRM_ERROR("Unable to add secondary DMA buffers: %d\n", err); return err; } @@ -534,7 +540,7 @@ static int mga_do_agp_dma_bootstrap(drm_device_t * dev, err = drm_addmap( dev, offset, agp_size - offset, _DRM_AGP, 0, & dev_priv->agp_textures ); if (err) { - DRM_ERROR("Unable to map AGP texture region\n"); + DRM_ERROR("Unable to map AGP texture region: %d\n", err); return err; } @@ -575,7 +581,7 @@ static int mga_do_pci_dma_bootstrap(drm_device_t * dev, drm_mga_dma_bootstrap_t * dma_bs) { drm_mga_private_t * const dev_priv = (drm_mga_private_t *) dev->dev_private; - const unsigned int warp_size = mga_warp_microcode_size(dev_priv); + unsigned int warp_size = mga_warp_microcode_size(dev_priv); unsigned int primary_size; unsigned int bin_count; int err; @@ -587,11 +593,18 @@ static int mga_do_pci_dma_bootstrap(drm_device_t * dev, return DRM_ERR(EFAULT); } + /* Make drm_addbufs happy by not trying to create a mapping for less + * than a page. + */ + if (warp_size < PAGE_SIZE) + warp_size = PAGE_SIZE; + /* The proper alignment is 0x100 for this mapping */ err = drm_addmap(dev, 0, warp_size, _DRM_CONSISTENT, _DRM_READ_ONLY, &dev_priv->warp); if (err != 0) { - DRM_ERROR("Unable to create mapping for WARP microcode\n"); + DRM_ERROR("Unable to create mapping for WARP microcode: %d\n", + err); return err; } @@ -611,7 +624,7 @@ static int mga_do_pci_dma_bootstrap(drm_device_t * dev, } if (err != 0) { - DRM_ERROR("Unable to allocate primary DMA region\n"); + DRM_ERROR("Unable to allocate primary DMA region: %d\n", err); return DRM_ERR(ENOMEM); } @@ -636,7 +649,7 @@ static int mga_do_pci_dma_bootstrap(drm_device_t * dev, } if (bin_count == 0) { - DRM_ERROR("Unable to add secondary DMA buffers\n"); + DRM_ERROR("Unable to add secondary DMA buffers: %d\n", err); return err; } @@ -674,7 +687,7 @@ static int mga_do_dma_bootstrap(drm_device_t * dev, err = drm_addmap( dev, dev_priv->mmio_base, dev_priv->mmio_size, _DRM_REGISTERS, _DRM_READ_ONLY, & dev_priv->mmio ); if (err) { - DRM_ERROR("Unable to map MMIO region\n"); + DRM_ERROR("Unable to map MMIO region: %d\n", err); return err; } @@ -683,7 +696,7 @@ static int mga_do_dma_bootstrap(drm_device_t * dev, _DRM_READ_ONLY | _DRM_LOCKED | _DRM_KERNEL, & dev_priv->status ); if (err) { - DRM_ERROR("Unable to map status region\n"); + DRM_ERROR("Unable to map status region: %d\n", err); return err; } @@ -845,14 +858,14 @@ static int mga_do_init_dma(drm_device_t * dev, drm_mga_init_t * init) } ret = mga_warp_install_microcode(dev_priv); - if (ret < 0) { - DRM_ERROR("failed to install WARP ucode!\n"); + if (ret != 0) { + DRM_ERROR("failed to install WARP ucode: %d!\n", ret); return ret; } ret = mga_warp_init(dev_priv); - if (ret < 0) { - DRM_ERROR("failed to init WARP engine!\n"); + if (ret != 0) { + DRM_ERROR("failed to init WARP engine: %d!\n", ret); return ret; } -- cgit v1.2.3