Some additional changes from jhb:

- It now initializes the buffer to a known-good state (length of 0) so that
  you can't do a buffer overrun by reading it without writing to it.
- It doesn't include the trailing null character in 'len' and always leaves
  room for it during writes by restricting writes to writing only 255 chars,
  but letting reads read 256 chars.  This means after init you can do a read
  of one byte to get an empty string, and if you write "foo" (3 bytes) you
  can read back "foo\0" (I think this is the original intent).  Letting
  'len' not hold the null simplifies a fair bit of logic in write.
- Use 'td' for thread pointers, not 'p' (which is from when this was a
  'struct proc *').
- Some style fixes.
- Don't ever set uio_offset directly, uiomove() already changes it, and in
  fact we should read it after the write to handle partial writes.

Submitted by:	jhb
Approved by:	bcr (mentor, implicit)
This commit is contained in:
Eitan Adler 2013-03-17 02:06:36 +00:00
parent 3b00475796
commit fbbf024e83
Notes: svn2git 2020-12-08 03:00:23 +00:00
svn path=/head/; revision=41250

View file

@ -188,7 +188,7 @@ KMOD=skeleton
#include <sys/uio.h> /* uio struct */
#include <sys/malloc.h>
#define BUFFERSIZE 256
#define BUFFERSIZE 255
/* Function prototypes */
static d_open_t echo_open;
@ -207,7 +207,7 @@ static struct cdevsw echo_cdevsw = {
};
struct s_echo {
char msg[BUFFERSIZE];
char msg[BUFFERSIZE + 1];
int len;
};
@ -222,7 +222,6 @@ MALLOC_DEFINE(M_ECHOBUF, "echobuffer", "buffer for echo module");
* This function is called by the kld[un]load(2) system calls to
* determine what actions to take when a module is loaded or unloaded.
*/
static int
echo_loader(struct module *m __unused, int what, void *arg __unused)
{
@ -241,8 +240,8 @@ echo_loader(struct module *m __unused, int what, void *arg __unused)
if (error != 0)
break;
/* kmalloc memory for use by this driver */
echomsg = malloc(sizeof(*echomsg), M_ECHOBUF, M_WAITOK);
echomsg = malloc(sizeof(*echomsg), M_ECHOBUF, M_WAITOK |
M_ZERO);
printf("Echo device loaded.\n");
break;
case MOD_UNLOAD:
@ -258,7 +257,8 @@ echo_loader(struct module *m __unused, int what, void *arg __unused)
}
static int
echo_open(struct cdev *dev __unused, int oflags __unused, int devtype __unused, struct thread *p __unused)
echo_open(struct cdev *dev __unused, int oflags __unused, int devtype __unused,
struct thread *td __unused)
{
int error = 0;
@ -267,7 +267,8 @@ echo_open(struct cdev *dev __unused, int oflags __unused, int devtype __unused,
}
static int
echo_close(struct cdev *dev __unused, int fflag __unused, int devtype __unused, struct thread *p __unused)
echo_close(struct cdev *dev __unused, int fflag __unused, int devtype __unused,
struct thread *td __unused)
{
uprintf("Closing device \"echo\".\n");
@ -279,19 +280,20 @@ echo_close(struct cdev *dev __unused, int fflag __unused, int devtype __unused,
* echo_write() and returns it to userland for accessing.
* uio(9)
*/
static int
echo_read(struct cdev *dev __unused, struct uio *uio, int ioflag __unused)
{
int error, amt;
size_t amt;
int error;
/*
* How big is this read operation? Either as big as the user wants,
* or as big as the remaining data
* or as big as the remaining data. Note that the 'len' does not
* include the trailing null character.
*/
amt = MIN(uio->uio_resid, uio->uio_offset >= echomsg->len + 1 ? 0 :
echomsg->len + 1 - uio->uio_offset);
amt = MIN(uio->uio_resid, echomsg->len - uio->uio_offset);
uio->uio_offset += amt;
if ((error = uiomove(echomsg->msg, amt, uio)) != 0)
uprintf("uiomove failed!\n");
@ -302,13 +304,11 @@ echo_read(struct cdev *dev __unused, struct uio *uio, int ioflag __unused)
* echo_write takes in a character string and saves it
* to buf for later accessing.
*/
static int
echo_write(struct cdev *dev __unused, struct uio *uio, int ioflag __unused)
{
int error, amt;
/* Copy the string in from user memory to kernel memory */
size_t amt;
int error;
/*
* We either write from the beginning or are appending -- do
@ -317,32 +317,25 @@ echo_write(struct cdev *dev __unused, struct uio *uio, int ioflag __unused)
if (uio->uio_offset != 0 && (uio->uio_offset != echomsg->len))
return (EINVAL);
/*
* This is new message, reset length
*/
/* This is a new message, reset length */
if (uio->uio_offset == 0)
echomsg->len = 0;
/* NULL character should be overridden */
if (echomsg->len != 0)
echomsg->len--;
/* Copy the string in from user memory to kernel memory */
amt = MIN(uio->uio_resid, (BUFFERSIZE - echomsg->len));
error = uiomove(echomsg->msg + uio->uio_offset, amt, uio);
/* Now we need to null terminate, then record the length */
echomsg->len += amt + 1;
uio->uio_offset += amt + 1;
echomsg->msg[echomsg->len - 1] = 0;
/* Now we need to null terminate and record the length */
echomsg->len = uio->uio_offset;
echomsg->msg[echomsg->len] = 0;
if (error != 0)
uprintf("Write failed: bad address!\n");
return (error);
}
DEV_MODULE(echo,echo_loader,NULL);</programlisting>
DEV_MODULE(echo, echo_loader, NULL);</programlisting>
</example>
<para>With this driver loaded try:</para>