[PATCH 1/2] ppc64: fix read/write on large /dev/nvram

[Arnd Bergmann]

On Dinsdag 28 Juni 2005 11:36, Paul Mackerras wrote:
> Hmmm, I'm not sure that always using __get_free_page even for small
> reads is necessarily a good idea.  If we are just limiting the size of
> the read or write to PAGE_SIZE we might as well just limit the count
> to PAGE_SIZE and continue to use kmalloc.  I think that we should
> probably limit the buffer size but then loop around until the whole
> user request is satisfied.

IIRC, the tools we commonly use to access nvram first check the
size and then try to get the whole buffer in a single read request.
This means that we will still use full pages most of the time with
kmalloc, so it shouldn't make any difference one way or the other.

Anyway, here is an updated patch using kmalloc and with a fix for
the bug noticed by Milton.

---

The generic ppc64 nvram code breaks when passed large buffers
to read/write because of the size limit for kmalloc. This
patch fixes this by allocating at most PAGE_SIZE bytes.
It also cleans up the error handling for those function
so they become more readable.

Signed-off-by: Arnd Bergmann <arnd at arndb.de>

--- linux-cg.orig/arch/ppc64/kernel/nvram.c	2005-06-28 13:14:04.275989864 -0400
+++ linux-cg/arch/ppc64/kernel/nvram.c	2005-06-28 13:40:28.377013664 -0400
@@ -81,80 +81,74 @@ static loff_t dev_nvram_llseek(struct fi
 static ssize_t dev_nvram_read(struct file *file, char __user *buf,
 			  size_t count, loff_t *ppos)
 {
-	ssize_t len;
-	char *tmp_buffer;
-	int size;
+	ssize_t ret;
+	char *tmp = NULL;
+	ssize_t size;
+
+	ret = -ENODEV;
+	if (!ppc_md.nvram_size)
+		goto out;
 
-	if (ppc_md.nvram_size == NULL)
-		return -ENODEV;
+	ret = 0;
 	size = ppc_md.nvram_size();
+	if (*ppos >= size || size < 0)
+		goto out;
 
-	if (!access_ok(VERIFY_WRITE, buf, count))
-		return -EFAULT;
-	if (*ppos >= size)
-		return 0;
-	if (count > size) 
-		count = size;
-
-	tmp_buffer = (char *) kmalloc(count, GFP_KERNEL);
-	if (!tmp_buffer) {
-		printk(KERN_ERR "dev_read_nvram: kmalloc failed\n");
-		return -ENOMEM;
-	}
-
-	len = ppc_md.nvram_read(tmp_buffer, count, ppos);
-	if ((long)len <= 0) {
-		kfree(tmp_buffer);
-		return len;
-	}
-
-	if (copy_to_user(buf, tmp_buffer, len)) {
-		kfree(tmp_buffer);
-		return -EFAULT;
-	}
+	count = min_t(size_t, count, size - *ppos);
+	count = min(count, PAGE_SIZE);
 
-	kfree(tmp_buffer);
-	return len;
+	ret = -ENOMEM;
+	tmp = (char *) __get_free_page(GFP_KERNEL);
+	if (!tmp)
+		goto out;
+
+	ret = ppc_md.nvram_read(tmp, count, ppos);
+	if (ret <= 0)
+		goto out;
+
+	if (copy_to_user(buf, tmp, ret))
+		ret = -EFAULT;
+
+out:
+	free_page((unsigned long)tmp);
+	return ret;
 
 }
 
 static ssize_t dev_nvram_write(struct file *file, const char __user *buf,
-			   size_t count, loff_t *ppos)
+			  size_t count, loff_t *ppos)
 {
-	ssize_t len;
-	char * tmp_buffer;
-	int size;
+	ssize_t ret;
+	char *tmp = NULL;
+	ssize_t size;
+
+	ret = -ENODEV;
+	if (!ppc_md.nvram_size)
+		goto out;
 
-	if (ppc_md.nvram_size == NULL)
-		return -ENODEV;
+	ret = 0;
 	size = ppc_md.nvram_size();
+	if (*ppos >= size || size < 0)
+		goto out;
 
-	if (!access_ok(VERIFY_READ, buf, count))
-		return -EFAULT;
-	if (*ppos >= size)
-		return 0;
-	if (count > size)
-		count = size;
-
-	tmp_buffer = (char *) kmalloc(count, GFP_KERNEL);
-	if (!tmp_buffer) {
-		printk(KERN_ERR "dev_nvram_write: kmalloc failed\n");
-		return -ENOMEM;
-	}
-	
-	if (copy_from_user(tmp_buffer, buf, count)) {
-		kfree(tmp_buffer);
-		return -EFAULT;
-	}
+	count = min_t(size_t, count, size - *ppos);
+	count = min(count, PAGE_SIZE);
 
-	len = ppc_md.nvram_write(tmp_buffer, count, ppos);
-	if ((long)len <= 0) {
-		kfree(tmp_buffer);
-		return len;
-	}
+	ret = -ENOMEM;
+	tmp = (char *) __get_free_page(GFP_KERNEL);
+	if (!tmp)
+		goto out;
+
+	ret = -EFAULT;
+	if (copy_from_user(tmp, buf, count))
+		goto out;
+
+	ret = ppc_md.nvram_write(tmp, count, ppos);
+
+out:
+	free_page((unsigned long)tmp);
+	return ret;
 
-	kfree(tmp_buffer);
-	return len;
 }
 
 static int dev_nvram_ioctl(struct inode *inode, struct file *file,