Here is a patch that improves (I hope) the documentation
of how user proc-file-read functions should interface with
proc_file_read(). At this stage I just ask that any
interested parties review what I have written. // Thomas
--- linux-2.4.18-pre7_ORIG/fs/proc/generic.c Fri Sep 7 13:53:59 2001
+++ linux-2.4.18-pre7/fs/proc/generic.c Sat Feb 2 09:45:14 2002
@@ -67,23 +67,60 @@
start = NULL;
if (dp->get_info) {
- /*
- * Handle backwards compatibility with the old net
- * routines.
- */
+ /* Handle old net routines */
n = dp->get_info(page, &start, *ppos, count);
if (n < count)
eof = 1;
} else if (dp->read_proc) {
+ /*
+ * How to be a proc read function
+ * ------------------------------
+ *
+ * Prototype:
+ * int f(char *buffer, char **start, off_t offset,
+ * int count, int *peof, void *dat)
+ *
+ * You may assume that the buffer provided is at least
+ * 3 KiB in size.
+ *
+ * If you know you have supplied all the data there is,
+ * set the *peof bit.
+ *
+ * Three ways to return data:
+ * A) Set *start = NULL
+ * Put the data of the requested offset at that
+ * offset within the buffer. Return the number (n)
+ * of bytes there are from the beginning of the
+ * buffer up to the last byte of data. If the
+ * number of supplied bytes (= n - offset) is less
+ * than the requested count, you will be called
+ * again with the requested offset advanced by n.
+ * This interface is useful for files no larger
+ * than the buffer.
+ * B) Set *start = a small value (less than buffer!)
+ * Put the data of the requested offset at the
+ * beginning of the buffer. Return the number of
+ * bytes of data placed there. If this number is
+ * less than the requested count, you will be
+ * called again with the requested offset advanced
+ * by *start. This interface is useful when you
+ * have a large file consisting of a series of
+ * data blocks which you want to count and return
+ * as wholes.
+ * (hack by [email protected])
+ * C) Set *start = a location within the buffer
+ * Put the data of the requested offset at *start.
+ * Return the number (n) of bytes of data placed
+ * there. If this number is less than the
+ * requested count, you will be called again with
+ * the requested offset advanced by n.
+ */
n = dp->read_proc(page, &start, *ppos,
count, &eof, dp->data);
} else
break;
- if (!start) {
- /*
- * For proc files that are less than 4k
- */
+ if (start == NULL) {
start = page + *ppos;
n -= *ppos;
if (n <= 0)
@@ -91,19 +128,14 @@
if (n > count)
n = count;
}
- if (n == 0)
- break; /* End of file */
- if (n < 0) {
+ if (n == 0) /* end of file */
+ break;
+ if (n < 0) { /* error */
if (retval == 0)
retval = n;
break;
}
- /* This is a hack to allow mangling of file pos independent
- * of actual bytes read. Simply place the data at page,
- * return the bytes, and set `start' to the desired offset
- * as an unsigned int. - [email protected]
- */
n -= copy_to_user(buf, start < page ? page : start, n);
if (n == 0) {
if (retval == 0)
@@ -111,7 +143,7 @@
break;
}
- *ppos += start < page ? (long)start : n; /* Move down the file */
+ *ppos += start < page ? (long)start : n;
nbytes -= n;
buf += n;
retval += n;
After further study I have revised the documentation
to fix some mistakes.
This patch also fixes some problems in the code. First,
the patch adds checks for buffer overruns. (The
original code allows the user to return any number
as the number of bytes to copy to the user, even if
this would exceed procfs's and/or the user's buffer.)
Second, with *start==NULL, n is trimmed down to "count"
_prior_ to subtracting the offset. (The original code
trims n down to count after subtracting the offset;
but this means that it could be copying up to count
many bytes near the very end of the buffer.)
Still just asking for criticism.
--
Thomas
--- linux-2.4.18-pre7_ORIG/fs/proc/generic.c Fri Sep 7 13:53:59 2001
+++ linux-2.4.18-pre7/fs/proc/generic.c Sun Feb 3 08:51:32 2002
@@ -65,55 +65,101 @@
{
count = MIN(PROC_BLOCK_SIZE, nbytes);
start = NULL;
if (dp->get_info) {
- /*
- * Handle backwards compatibility with the old net
- * routines.
- */
+ /* Handle old net routines */
n = dp->get_info(page, &start, *ppos, count);
if (n < count)
eof = 1;
} else if (dp->read_proc) {
+ /*
+ * How to be a proc read function
+ * ------------------------------
+ * Prototype:
+ * int f(char *buffer, char **start, off_t offset,
+ * int count, int *peof, void *dat)
+ *
+ * Assume that the buffer is "count" bytes in size.
+ *
+ * If you know you have supplied all the data you
+ * have, set *peof.
+ *
+ * You have three ways to return data:
+ * 0) Leave *start = NULL. (This is the default.)
+ * Put the data of the requested offset at that
+ * offset within the buffer. Return the number (n)
+ * of bytes there are from the beginning of the
+ * buffer up to the last byte of data. If the
+ * number of supplied bytes (= n - offset) is
+ * greater than zero (and you didn't signal eof)
+ * you will be called again with the requested
+ * offset advanced by the number of bytes
+ * absorbed. This interface is useful for files
+ * no larger than the buffer.
+ * 1) Set *start = a small value (less than buffer
+ * but greater than zero).
+ * Put the data of the requested offset at the
+ * beginning of the buffer. Return the number of
+ * bytes of data placed there. If this number is
+ * greater than zero (and you didn't signal eof),
+ * you will be called again with the requested
+ * offset advanced by *start. This interface is
+ * useful when you have a large file consisting
+ * of a series of blocks which you want to count
+ * and return as wholes.
+ * (hack by [email protected])
+ * 2) Set *start = an address within the buffer.
+ * Put the data of the requested offset at *start.
+ * Return the number (n) of bytes of data placed
+ * there. If this number is greater than zero
+ * (and you didn't signal eof) you will be called
+ * again with the requested offset advanced by
+ * the number of bytes absorbed.
+ */
n = dp->read_proc(page, &start, *ppos,
count, &eof, dp->data);
} else
break;
- if (!start) {
- /*
- * For proc files that are less than 4k
- */
+ if (n == 0) /* end of file */
+ break;
+ if (n < 0) { /* error */
+ if (retval == 0)
+ retval = n;
+ break;
+ }
+
+ if (start == NULL) {
start = page + *ppos;
+ if (n > PAGE_SIZE)
+ printk(KERN_ERR "proc_file_read: Buffer overflow!\n");
+ if (n > count)
+ n = count;
n -= *ppos;
if (n <= 0)
break;
+ } else if (start < page) {
+ if (n > PAGE_SIZE)
+ printk(KERN_ERR "proc_file_read: Buffer overflow!\n");
+ if (n > count)
+ printk(KERN_WARNING "proc_file_read: Read count exceeded\n");
+ } else /* start >= page */ {
+ if ((start - page + n) > PAGE_SIZE)
+ printk(KERN_ERR "proc_file_read: Buffer overflow!\n");
if (n > count)
n = count;
}
- if (n == 0)
- break; /* End of file */
- if (n < 0) {
- if (retval == 0)
- retval = n;
- break;
- }
- /* This is a hack to allow mangling of file pos independent
- * of actual bytes read. Simply place the data at page,
- * return the bytes, and set `start' to the desired offset
- * as an unsigned int. - [email protected]
- */
n -= copy_to_user(buf, start < page ? page : start, n);
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}
- *ppos += start < page ? (long)start : n; /* Move down the file */
+ *ppos += start < page ? (unsigned long)start : n;
nbytes -= n;
buf += n;
retval += n;
}
free_page((unsigned long) page);
Here's the final version of the patch to add documentary
comments and buffer overflow checking to the proc_file_read()
function.
--- linux-2.4.18-pre8_ORIG/fs/proc/generic.c Fri Sep 7 13:53:59 2001
+++ linux-2.4.18-pre8/fs/proc/generic.c Wed Feb 6 09:26:25 2002
@@ -65,55 +65,120 @@
{
count = MIN(PROC_BLOCK_SIZE, nbytes);
start = NULL;
if (dp->get_info) {
- /*
- * Handle backwards compatibility with the old net
- * routines.
- */
+ /* Handle old net routines */
n = dp->get_info(page, &start, *ppos, count);
if (n < count)
eof = 1;
} else if (dp->read_proc) {
+ /*
+ * How to be a proc read function
+ * ------------------------------
+ * Prototype:
+ * int f(char *buffer, char **start, off_t offset,
+ * int count, int *peof, void *dat)
+ *
+ * Assume that the buffer is "count" bytes in size.
+ *
+ * If you know you have supplied all the data you
+ * have, set *peof.
+ *
+ * You have three ways to return data:
+ * 0) Leave *start = NULL. (This is the default.)
+ * Put the data of the requested offset at that
+ * offset within the buffer. Return the number (n)
+ * of bytes there are from the beginning of the
+ * buffer up to the last byte of data. If the
+ * number of supplied bytes (= n - offset) is
+ * greater than zero and you didn't signal eof
+ * and the reader is prepared to take more data
+ * you will be called again with the requested
+ * offset advanced by the number of bytes
+ * absorbed. This interface is useful for files
+ * no larger than the buffer.
+ * 1) Set *start = an unsigned long value less than
+ * the buffer address but greater than zero.
+ * Put the data of the requested offset at the
+ * beginning of the buffer. Return the number of
+ * bytes of data placed there. If this number is
+ * greater than zero and you didn't signal eof
+ * and the reader is prepared to take more data
+ * you will be called again with the requested
+ * offset advanced by *start. This interface is
+ * useful when you have a large file consisting
+ * of a series of blocks which you want to count
+ * and return as wholes.
+ * (Hack by [email protected])
+ * 2) Set *start = an address within the buffer.
+ * Put the data of the requested offset at *start.
+ * Return the number of bytes of data placed there.
+ * If this number is greater than zero and you
+ * didn't signal eof and the reader is prepared to
+ * take more data you will be called again with the
+ * requested offset advanced by the number of bytes
+ * absorbed.
+ */
n = dp->read_proc(page, &start, *ppos,
count, &eof, dp->data);
} else
break;
- if (!start) {
- /*
- * For proc files that are less than 4k
- */
- start = page + *ppos;
+ if (n == 0) /* end of file */
+ break;
+ if (n < 0) { /* error */
+ if (retval == 0)
+ retval = n;
+ break;
+ }
+
+ if (start == NULL) {
+ if (n > PAGE_SIZE) {
+ printk(KERN_ERR
+ "proc_file_read: Apparent buffer overflow!\n");
+ n = PAGE_SIZE;
+ }
+ if (n > count)
+ n = count;
n -= *ppos;
if (n <= 0)
break;
+ start = page + *ppos;
+ } else if (start < page) {
+ if (n > PAGE_SIZE) {
+ printk(KERN_ERR
+ "proc_file_read: Apparent buffer overflow!\n");
+ n = PAGE_SIZE;
+ }
+ if (n > count) {
+ /*
+ * Don't reduce n because doing so might
+ * cut off part of a data block.
+ */
+ printk(KERN_WARNING
+ "proc_file_read: Read count exceeded\n");
+ }
+ } else /* start >= page */ {
+ unsigned long startoff = (unsigned long)(start - page);
+ if (n > (PAGE_SIZE - startoff)) {
+ printk(KERN_ERR
+ "proc_file_read: Apparent buffer overflow!\n");
+ n = PAGE_SIZE - startoff;
+ }
if (n > count)
n = count;
}
- if (n == 0)
- break; /* End of file */
- if (n < 0) {
- if (retval == 0)
- retval = n;
- break;
- }
- /* This is a hack to allow mangling of file pos independent
- * of actual bytes read. Simply place the data at page,
- * return the bytes, and set `start' to the desired offset
- * as an unsigned int. - [email protected]
- */
n -= copy_to_user(buf, start < page ? page : start, n);
if (n == 0) {
if (retval == 0)
retval = -EFAULT;
break;
}
- *ppos += start < page ? (long)start : n; /* Move down the file */
+ *ppos += start < page ? (unsigned long)start : n;
nbytes -= n;
buf += n;
retval += n;
}
free_page((unsigned long) page);