ia64/linux-2.6.18-xen.hg

changeset 863:464a925d73f1

blktap: don't access deallocated data

Dereferencing filp->private_data->vma in the file_operations.release
actor isn't permitted, as the vma generally has been destroyed by that
time. The kfree()ing of vma->vm_private_data must be done in the
vm_operations.close actor, and the call to zap_page_range() seems
redundant with the caller of that actor altogether.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
author Keir Fraser <keir.fraser@citrix.com>
date Fri Apr 17 13:03:22 2009 +0100 (2009-04-17)
parents dfd2adc58740
children 613216635ff0
files drivers/xen/blktap/blktap.c
line diff
     1.1 --- a/drivers/xen/blktap/blktap.c	Thu Apr 16 11:47:44 2009 +0100
     1.2 +++ b/drivers/xen/blktap/blktap.c	Fri Apr 17 13:03:22 2009 +0100
     1.3 @@ -293,6 +293,10 @@ static inline int OFFSET_TO_SEG(int offs
     1.4  /******************************************************************
     1.5   * BLKTAP VM OPS
     1.6   */
     1.7 +struct tap_vma_priv {
     1.8 +	tap_blkif_t *info;
     1.9 +	struct page *map[];
    1.10 +};
    1.11  
    1.12  static struct page *blktap_nopage(struct vm_area_struct *vma,
    1.13  				  unsigned long address,
    1.14 @@ -315,7 +319,7 @@ static pte_t blktap_clear_pte(struct vm_
    1.15  	int offset, seg, usr_idx, pending_idx, mmap_idx;
    1.16  	unsigned long uvstart = vma->vm_start + (RING_PAGES << PAGE_SHIFT);
    1.17  	unsigned long kvaddr;
    1.18 -	struct page **map;
    1.19 +	struct tap_vma_priv *priv;
    1.20  	struct page *pg;
    1.21  	struct grant_handle_pair *khandle;
    1.22  	struct gnttab_unmap_grant_ref unmap[2];
    1.23 @@ -330,12 +334,12 @@ static pte_t blktap_clear_pte(struct vm_
    1.24  					       ptep, is_fullmm);
    1.25  
    1.26  	info = vma->vm_file->private_data;
    1.27 -	map = vma->vm_private_data;
    1.28 +	priv = vma->vm_private_data;
    1.29  
    1.30  	/* TODO Should these be changed to if statements? */
    1.31  	BUG_ON(!info);
    1.32  	BUG_ON(!info->idx_map);
    1.33 -	BUG_ON(!map);
    1.34 +	BUG_ON(!priv);
    1.35  
    1.36  	offset = (int) ((uvaddr - uvstart) >> PAGE_SHIFT);
    1.37  	usr_idx = OFFSET_TO_USR_IDX(offset);
    1.38 @@ -347,7 +351,7 @@ static pte_t blktap_clear_pte(struct vm_
    1.39  	kvaddr = idx_to_kaddr(mmap_idx, pending_idx, seg);
    1.40  	pg = pfn_to_page(__pa(kvaddr) >> PAGE_SHIFT);
    1.41  	ClearPageReserved(pg);
    1.42 -	map[offset + RING_PAGES] = NULL;
    1.43 +	priv->map[offset + RING_PAGES] = NULL;
    1.44  
    1.45  	khandle = &pending_handle(mmap_idx, pending_idx, seg);
    1.46  
    1.47 @@ -388,9 +392,20 @@ static pte_t blktap_clear_pte(struct vm_
    1.48  	return copy;
    1.49  }
    1.50  
    1.51 +static void blktap_vma_close(struct vm_area_struct *vma)
    1.52 +{
    1.53 +	struct tap_vma_priv *priv = vma->vm_private_data;
    1.54 +
    1.55 +	if (priv) {
    1.56 +		priv->info->vma = NULL;
    1.57 +		kfree(priv);
    1.58 +	}
    1.59 +}
    1.60 +
    1.61  struct vm_operations_struct blktap_vm_ops = {
    1.62  	nopage:   blktap_nopage,
    1.63  	zap_pte:  blktap_clear_pte,
    1.64 +	close:    blktap_vma_close,
    1.65  };
    1.66  
    1.67  /******************************************************************
    1.68 @@ -609,21 +624,6 @@ static int blktap_release(struct inode *
    1.69  	ClearPageReserved(virt_to_page(info->ufe_ring.sring));
    1.70  	free_page((unsigned long) info->ufe_ring.sring);
    1.71  
    1.72 -	/* Clear any active mappings and free foreign map table */
    1.73 -	if (info->vma) {
    1.74 -		struct mm_struct *mm = info->vma->vm_mm;
    1.75 -
    1.76 -		down_write(&mm->mmap_sem);
    1.77 -		zap_page_range(
    1.78 -			info->vma, info->vma->vm_start, 
    1.79 -			info->vma->vm_end - info->vma->vm_start, NULL);
    1.80 -		up_write(&mm->mmap_sem);
    1.81 -
    1.82 -		kfree(info->vma->vm_private_data);
    1.83 -
    1.84 -		info->vma = NULL;
    1.85 -	}
    1.86 -
    1.87  	if (info->idx_map) {
    1.88  		kfree(info->idx_map);
    1.89  		info->idx_map = NULL;
    1.90 @@ -662,8 +662,7 @@ static int blktap_release(struct inode *
    1.91  static int blktap_mmap(struct file *filp, struct vm_area_struct *vma)
    1.92  {
    1.93  	int size;
    1.94 -	struct page **map;
    1.95 -	int i;
    1.96 +	struct tap_vma_priv *priv;
    1.97  	tap_blkif_t *info = filp->private_data;
    1.98  	int ret;
    1.99  
   1.100 @@ -700,18 +699,16 @@ static int blktap_mmap(struct file *filp
   1.101  	}
   1.102  
   1.103  	/* Mark this VM as containing foreign pages, and set up mappings. */
   1.104 -	map = kzalloc(((vma->vm_end - vma->vm_start) >> PAGE_SHIFT)
   1.105 -		      * sizeof(struct page *),
   1.106 -		      GFP_KERNEL);
   1.107 -	if (map == NULL) {
   1.108 +	priv = kzalloc(sizeof(*priv) + ((vma->vm_end - vma->vm_start)
   1.109 +					>> PAGE_SHIFT) * sizeof(*priv->map),
   1.110 +		       GFP_KERNEL);
   1.111 +	if (priv == NULL) {
   1.112  		WPRINTK("Couldn't alloc VM_FOREIGN map.\n");
   1.113  		goto fail;
   1.114  	}
   1.115 +	priv->info = info;
   1.116  
   1.117 -	for (i = 0; i < ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT); i++)
   1.118 -		map[i] = NULL;
   1.119 -    
   1.120 -	vma->vm_private_data = map;
   1.121 +	vma->vm_private_data = priv;
   1.122  	vma->vm_flags |= VM_FOREIGN;
   1.123  	vma->vm_flags |= VM_DONTCOPY;
   1.124  
   1.125 @@ -1190,7 +1187,7 @@ static int blktap_read_ufe_ring(tap_blki
   1.126  		for (j = 0; j < pending_req->nr_pages; j++) {
   1.127  
   1.128  			unsigned long kvaddr, uvaddr;
   1.129 -			struct page **map = info->vma->vm_private_data;
   1.130 +			struct tap_vma_priv *priv = info->vma->vm_private_data;
   1.131  			struct page *pg;
   1.132  			int offset;
   1.133  
   1.134 @@ -1201,7 +1198,7 @@ static int blktap_read_ufe_ring(tap_blki
   1.135  			ClearPageReserved(pg);
   1.136  			offset = (uvaddr - info->vma->vm_start) 
   1.137  				>> PAGE_SHIFT;
   1.138 -			map[offset] = NULL;
   1.139 +			priv->map[offset] = NULL;
   1.140  		}
   1.141  		fast_flush_area(pending_req, pending_idx, usr_idx, info->minor);
   1.142  		info->idx_map[usr_idx] = INVALID_REQ;
   1.143 @@ -1359,6 +1356,7 @@ static void dispatch_rw_block_io(blkif_t
   1.144  	unsigned int nseg;
   1.145  	int ret, i, nr_sects = 0;
   1.146  	tap_blkif_t *info;
   1.147 +	struct tap_vma_priv *priv;
   1.148  	blkif_request_t *target;
   1.149  	int pending_idx = RTN_PEND_IDX(pending_req,pending_req->mem_idx);
   1.150  	int usr_idx;
   1.151 @@ -1407,6 +1405,7 @@ static void dispatch_rw_block_io(blkif_t
   1.152  	pending_req->status    = BLKIF_RSP_OKAY;
   1.153  	pending_req->nr_pages  = nseg;
   1.154  	op = 0;
   1.155 +	priv = info->vma->vm_private_data;
   1.156  	mm = info->vma->vm_mm;
   1.157  	if (!xen_feature(XENFEAT_auto_translated_physmap))
   1.158  		down_write(&mm->mmap_sem);
   1.159 @@ -1490,8 +1489,7 @@ static void dispatch_rw_block_io(blkif_t
   1.160  							  >> PAGE_SHIFT));
   1.161  			offset = (uvaddr - info->vma->vm_start) >> PAGE_SHIFT;
   1.162  			pg = pfn_to_page(__pa(kvaddr) >> PAGE_SHIFT);
   1.163 -			((struct page **)info->vma->vm_private_data)[offset] =
   1.164 -				pg;
   1.165 +			priv->map[offset] = pg;
   1.166  		}
   1.167  	} else {
   1.168  		for (i = 0; i < nseg; i++) {
   1.169 @@ -1518,8 +1516,7 @@ static void dispatch_rw_block_io(blkif_t
   1.170  
   1.171  			offset = (uvaddr - info->vma->vm_start) >> PAGE_SHIFT;
   1.172  			pg = pfn_to_page(__pa(kvaddr) >> PAGE_SHIFT);
   1.173 -			((struct page **)info->vma->vm_private_data)[offset] =
   1.174 -				pg;
   1.175 +			priv->map[offset] = pg;
   1.176  		}
   1.177  	}
   1.178