You wouldn't believe a driver could survive doing userland IO without

properly using copyin/copyout for more than 5 years?  This one did. :-)

Properly encapsulate all user<->kernel data transfers using copy{in,out}.

MFC after:	1 month

2 files changed, 49 insertions, 12 deletions

diff --git a/sys/dev/smbus/smb.c b/sys/dev/smbus/smb.c
index 85ab273..47a159a 100644
--- a/sys/dev/smbus/smb.c
+++ b/sys/dev/smbus/smb.c
@@ -194,6 +194,9 @@ smbioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct thread *td)
 	device_t smbdev = IIC_DEVICE(minor(dev));
 	struct smb_softc *sc = IIC_SOFTC(minor(dev));
 	device_t parent = device_get_parent(smbdev);
+	char buf[SMB_MAXBLOCKSIZE];
+	char c;
+	short w;
 
 	int error = 0;
 	struct smbcmd *s = (struct smbcmd *)data;
@@ -234,37 +237,66 @@ smbioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct thread *td)
 		break;
 
 	case SMB_READB:
-		if (s->data.byte_ptr)
+		if (s->data.byte_ptr) {
 			error = smbus_error(smbus_readb(parent, s->slave,
-						s->cmd, s->data.byte_ptr));
+						s->cmd, &c));
+			if (error)
+				break;
+			error = copyout(&c, s->data.byte_ptr,
+					sizeof(*(s->data.byte_ptr)));
+		}
 		break;
 
 	case SMB_READW:
-		if (s->data.word_ptr)
+		if (s->data.word_ptr) {
 			error = smbus_error(smbus_readw(parent, s->slave,
-						s->cmd, s->data.word_ptr));
+						s->cmd, &w));
+			if (error == 0) {
+				error = copyout(&w, s->data.word_ptr,
+						sizeof(*(s->data.word_ptr)));
+			}
+		}
 		break;
 
 	case SMB_PCALL:
-		if (s->data.process.rdata)
+		if (s->data.process.rdata) {
+
 			error = smbus_error(smbus_pcall(parent, s->slave, s->cmd,
-				s->data.process.sdata, s->data.process.rdata));
+				s->data.process.sdata, &w));
+			if (error)
+				break;
+			error = copyout(&w, s->data.process.rdata,
+					sizeof(*(s->data.process.rdata)));
+		}
+		
 		break;
 
 	case SMB_BWRITE:
-		if (s->count && s->data.byte_ptr)
+		if (s->count && s->data.byte_ptr) {
+			if (s->count > SMB_MAXBLOCKSIZE)
+				s->count = SMB_MAXBLOCKSIZE;
+			error = copyin(s->data.byte_ptr, buf, s->count);
+			if (error)
+				break;
 			error = smbus_error(smbus_bwrite(parent, s->slave,
-						s->cmd, s->count, s->data.byte_ptr));
+						s->cmd, s->count, buf));
+		}
 		break;
 
 	case SMB_BREAD:
-		if (s->count && s->data.byte_ptr)
+		if (s->count && s->data.byte_ptr) {
+			if (s->count > SMB_MAXBLOCKSIZE)
+				s->count = SMB_MAXBLOCKSIZE;
 			error = smbus_error(smbus_bread(parent, s->slave,
-						s->cmd, s->count, s->data.byte_ptr));
+						s->cmd, s->count, buf));
+			if (error)
+				break;
+			error = copyout(buf, s->data.byte_ptr, s->count);
+		}
 		break;
 		
 	default:
-		error = ENODEV;
+		error = ENOTTY;
 	}
 
 	/* release the bus */
diff --git a/sys/dev/smbus/smb.h b/sys/dev/smbus/smb.h
index 7ed0a82..387515f 100644
--- a/sys/dev/smbus/smb.h
+++ b/sys/dev/smbus/smb.h
@@ -49,10 +49,15 @@ struct smbcmd {
 	} data;
 };
 
+/*
+ * SMBus spec 2.0 says block transfers may be at most 32 bytes.
+ */
+#define SMB_MAXBLOCKSIZE	32
+
 #define SMB_QUICK_WRITE	_IOW('i', 1, struct smbcmd)
 #define SMB_QUICK_READ	_IOW('i', 2, struct smbcmd)
 #define SMB_SENDB	_IOW('i', 3, struct smbcmd)
-#define SMB_RECVB	_IOW('i', 4, struct smbcmd)
+#define SMB_RECVB	_IOWR('i', 4, struct smbcmd)
 #define SMB_WRITEB	_IOW('i', 5, struct smbcmd)
 #define SMB_WRITEW	_IOW('i', 6, struct smbcmd)
 #define SMB_READB	_IOW('i', 7, struct smbcmd)