IMA: update ima_counts_put

- As ima_counts_put() may be called after the inode has been freed, verify that the inode is not NULL, before dereferencing it. - Maintain the IMA file counters in may_open() properly, decrementing any counter increments on subsequent errors. Reported-by: Ciprian Docan <docan@eden.rutgers.edu> Reported-by: J.R. Okajima <hooanon05@yahoo.co.jp> Signed-off-by: Mimi Zohar <zohar@us.ibm.com> Acked-by: Eric Paris <eparis@redhat.com Signed-off-by: James Morris <jmorris@namei.org>

IMA: update ima_counts_put
- As ima_counts_put() may be called after the inode has been freed, verify that the inode is not NULL, before dereferencing it. - Maintain the IMA file counters in may_open() properly, decrementing any counter increments on subsequent errors. Reported-by: Ciprian Docan <docan@eden.rutgers.edu> Reported-by: J.R. Okajima <hooanon05@yahoo.co.jp> Signed-off-by: Mimi Zohar <zohar@us.ibm.com> Acked-by: Eric Paris <eparis@redhat.com Signed-off-by: James Morris <jmorris@namei.org>
Mimi Zohar · James Morris
1 parent e07cccf404
Showing 2 changed files with 20 additions and 8 deletions Side-by-side Diff
fs/namei.c
security/integrity/ima/ima_main.c
@@ -1542,28 +1542,31 @@
 	 * An append-only file must be opened in append mode for writing.
 	 */
 	if (IS_APPEND(inode)) {
+		error = -EPERM;
 		if  ((flag & FMODE_WRITE) && !(flag & O_APPEND))
-			return -EPERM;
+			goto err_out;
 		if (flag & O_TRUNC)
-			return -EPERM;
+			goto err_out;
 	}
  
 	/* O_NOATIME can only be set by the owner or superuser */
 	if (flag & O_NOATIME)
-		if (!is_owner_or_cap(inode))
-			return -EPERM;
+		if (!is_owner_or_cap(inode)) {
+			error = -EPERM;
+			goto err_out;
+		}
  
 	/*
 	 * Ensure there are no outstanding leases on the file.
 	 */
 	error = break_lease(inode, flag);
 	if (error)
-		return error;
+		goto err_out;
  
 	if (flag & O_TRUNC) {
 		error = get_write_access(inode);
 		if (error)
-			return error;
+			goto err_out;
  
 		/*
 		 * Refuse to truncate files with mandatory locks held on them.
  
@@ -1581,12 +1584,17 @@
 		}
 		put_write_access(inode);
 		if (error)
-			return error;
+			goto err_out;
 	} else
 		if (flag & FMODE_WRITE)
 			vfs_dq_init(inode);
  
 	return 0;
+err_out:
+	ima_counts_put(path, acc_mode ?
+		       acc_mode & (MAY_READ | MAY_WRITE | MAY_EXEC) :
+		       ACC_MODE(flag) & (MAY_READ | MAY_WRITE));
+	return error;
 }
  
 /*
@@ -249,7 +249,11 @@
 	struct inode *inode = path->dentry->d_inode;
 	struct ima_iint_cache *iint;
  
-	if (!ima_initialized || !S_ISREG(inode->i_mode))
+	/* The inode may already have been freed, freeing the iint
+	 * with it. Verify the inode is not NULL before dereferencing
+	 * it.
+	 */
+	if (!ima_initialized || !inode || !S_ISREG(inode->i_mode))
 		return;
 	iint = ima_iint_find_insert_get(inode);
 	if (!iint)
...	...	@@ -1542,28 +1542,31 @@
1542	1542	* An append-only file must be opened in append mode for writing.
1543	1543	*/
1544	1544	if (IS_APPEND(inode)) {
	1545	+ error = -EPERM;
1545	1546	if ((flag & FMODE_WRITE) && !(flag & O_APPEND))
1546		- return -EPERM;
	1547	+ goto err_out;
1547	1548	if (flag & O_TRUNC)
1548		- return -EPERM;
	1549	+ goto err_out;
1549	1550	}
1550	1551
1551	1552	/* O_NOATIME can only be set by the owner or superuser */
1552	1553	if (flag & O_NOATIME)
1553		- if (!is_owner_or_cap(inode))
1554		- return -EPERM;
	1554	+ if (!is_owner_or_cap(inode)) {
	1555	+ error = -EPERM;
	1556	+ goto err_out;
	1557	+ }
1555	1558
1556	1559	/*
1557	1560	* Ensure there are no outstanding leases on the file.
1558	1561	*/
1559	1562	error = break_lease(inode, flag);
1560	1563	if (error)
1561		- return error;
	1564	+ goto err_out;
1562	1565
1563	1566	if (flag & O_TRUNC) {
1564	1567	error = get_write_access(inode);
1565	1568	if (error)
1566		- return error;
	1569	+ goto err_out;
1567	1570
1568	1571	/*
1569	1572	* Refuse to truncate files with mandatory locks held on them.
1570	1573
...	...	@@ -1581,12 +1584,17 @@
1581	1584	}
1582	1585	put_write_access(inode);
1583	1586	if (error)
1584		- return error;
	1587	+ goto err_out;
1585	1588	} else
1586	1589	if (flag & FMODE_WRITE)
1587	1590	vfs_dq_init(inode);
1588	1591
1589	1592	return 0;
	1593	+err_out:
	1594	+ ima_counts_put(path, acc_mode ?
	1595	+ acc_mode & (MAY_READ \| MAY_WRITE \| MAY_EXEC) :
	1596	+ ACC_MODE(flag) & (MAY_READ \| MAY_WRITE));
	1597	+ return error;
1590	1598	}
1591	1599
1592	1600	/*
...	...	@@ -249,7 +249,11 @@
249	249	struct inode *inode = path->dentry->d_inode;
250	250	struct ima_iint_cache *iint;
251	251
252		- if (!ima_initialized \|\| !S_ISREG(inode->i_mode))
	252	+ /* The inode may already have been freed, freeing the iint
	253	+ * with it. Verify the inode is not NULL before dereferencing
	254	+ * it.
	255	+ */
	256	+ if (!ima_initialized \|\| !inode \|\| !S_ISREG(inode->i_mode))
253	257	return;
254	258	iint = ima_iint_find_insert_get(inode);
255	259	if (!iint)