wacom: SMAP Fixes and Refactoring

Utilize user_memcpy and IS_USER_ADDRESS when necessary to prevent SMAP violations.
Also add a "wacom_device_header" struct to more easily share data between the wacom
kernel driver and input_server addon.

Should fix #14589

Change-Id: Ie2784020b21523f82fd450a2db2de60ccf9d6620
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2783
Reviewed-by: Adrien Destugues <[email protected]>

headers/private/input/wacom_driver.h

@@ -0,0 +1,19 @@
+/*
+ * Copyright 2020, Haiku, Inc. All Rights Reserved.
+ * Distributed under the terms of the MIT license.
+ */
+#ifndef WACOM_DRIVER_H
+#define WACOM_DRIVER_H
+
+
+#include <SupportDefs.h>
+
+
+typedef struct {
+	uint16	vendor_id;
+	uint16	product_id;
+	size_t	max_packet_size;
+} _PACKED wacom_device_header;
+
+
+#endif // WACOM_DRIVER_H

src/add-ons/input_server/devices/wacom/DeviceReader.cpp

@@ -1,7 +1,10 @@
 /*
- * Copyright 2005-2008 Stephan Aßmus <[email protected]>. All rights reserved.
+ * Copyright 2005-2008 Stephan Aßmus <[email protected]>
- * Distributed under the terms of the MIT license.
+ * Copyright 2020 Jacob Secunda <[email protected]>
+ * All rights reserved. Distributed under the terms of the MIT license.
  */
+
+
 #include "DeviceReader.h"
 
 #include <malloc.h>
@@ -9,19 +12,21 @@
 
 #include <File.h>
 
+#include <wacom_driver.h>
+
 #include "MasterServerDevice.h"
 
 
-static ssize_t kHeaderSize = 8;
+static ssize_t kHeaderSize = sizeof(wacom_device_header);
-
 
 // constructor
 DeviceReader::DeviceReader()
-	: fDevicePath(NULL),
+	:
-	  fDeviceFile(NULL),
+	fDevicePath(NULL),
-	  fVendorID(0),
+	fDeviceFile(NULL),
-	  fProductID(0),
+	fVendorID(0),
-	  fMaxPackedSize(0)
+	fProductID(0),
+	fMaxPacketSize(0)
 {
 }
 
@@ -36,23 +41,21 @@ status_t
 DeviceReader::SetTo(const char* path)
 {
 	status_t ret = B_BAD_VALUE;
-	if (path) {
+	if (path != NULL) {
 		_Unset();
 		fDevicePath = strdup(path);
 		fDeviceFile = new BFile(path, B_READ_ONLY);
 		ret = fDeviceFile->InitCheck();
 		if (ret >= B_OK) {
-			// read 8 bytes from the file and initialize
+			// read the wacom_device_header from the file and initialize
 			// the rest of the object variables
-			uint8 buffer[kHeaderSize];
+			wacom_device_header device_header;
-			ret = fDeviceFile->Read(buffer, kHeaderSize);
+			ret = fDeviceFile->Read(&device_header, kHeaderSize);
 			if (ret == kHeaderSize) {
 				ret = B_OK;
-				uint16* ids = (uint16*)buffer;
+				fVendorID = device_header.vendor_id;
-				fVendorID = ids[0];
+				fProductID = device_header.product_id;
-				fProductID = ids[1];
+				fMaxPacketSize = device_header.max_packet_size;
-				uint32* ps = (uint32*)buffer;
-				fMaxPackedSize = ps[1];
 			} else {
 				_Unset();
 			}
@@ -100,20 +103,20 @@ DeviceReader::ProductID() const
 size_t
 DeviceReader::MaxPacketSize() const
 {
-	return fMaxPackedSize;
+	return fMaxPacketSize;
 }
 
 // ReadData
 ssize_t
 DeviceReader::ReadData(uint8* data, const size_t size) const
 {
-	if (!fDeviceFile || fMaxPackedSize <= 0 || fMaxPackedSize > 128)
+	if (!fDeviceFile || fMaxPacketSize <= 0 || fMaxPacketSize > 128)
 		return B_NO_INIT;
 	status_t ret = fDeviceFile->InitCheck();
 	if (ret < B_OK)
 		return (ssize_t)ret;
 
-	ssize_t requested = fMaxPackedSize + kHeaderSize;
+	ssize_t requested = fMaxPacketSize + kHeaderSize;
 	uint8 buffer[requested];
 	ssize_t read = fDeviceFile->Read(buffer, requested);
 	if (read > kHeaderSize) {
@@ -153,5 +156,5 @@ DeviceReader::_Unset()
 	fDeviceFile = NULL;
 	fVendorID = 0;
 	fProductID = 0;
-	fMaxPackedSize = 0;
+	fMaxPacketSize = 0;
 }

src/add-ons/input_server/devices/wacom/DeviceReader.h

@@ -58,6 +58,6 @@ class DeviceReader {
 
 			uint16				fVendorID;
 			uint16				fProductID;
-			size_t				fMaxPackedSize;
+			size_t				fMaxPacketSize;
 };
 #endif // USB_DEVICE_MONITOR_H

src/add-ons/kernel/drivers/input/wacom/Jamfile

@@ -1,6 +1,8 @@
 SubDir HAIKU_TOP src add-ons kernel drivers input wacom ;
 
 SubDirSysHdrs $(HAIKU_TOP) headers os drivers ;
+UsePrivateHeaders input ;
+UsePrivateKernelHeaders ;
 
 KernelAddon wacom :
 	wacom.c

src/add-ons/kernel/drivers/input/wacom/wacom.c

@@ -1,5 +1,5 @@
 /*
- * Copyright 2005-2008, Haiku, Inc. All Rights Reserved.
+ * Copyright 2005-2020, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -18,6 +18,9 @@
 #include <OS.h>
 #include <USB3.h>
 
+#include <kernel.h>
+#include <wacom_driver.h>
+
 int32 api_version = B_CUR_DRIVER_API_VERSION;
 
 #define DEBUG_DRIVER 0
@@ -430,7 +433,8 @@ device_open(const char *dname, uint32 flags, void** cookie)
 				release_sem(sDeviceListLock);
 				return ret;
 //			} else {
-//				dprintf(ID "device_open() -> device is already open %ld\n", ret);
+//				dprintf(ID "device_open() -> device is already open %ld\n",
+//					ret);
 //				release_sem(sDeviceListLock);
 //				return B_ERROR;
 //			}
@@ -502,15 +506,20 @@ device_interupt_callback(void* cookie, status_t status, void* data,
 }
 
 // read_header
-static void
+static status_t
 read_header(const wacom_device* device, void* buffer)
 {
-	uint16* ids = (uint16*)buffer;
+	wacom_device_header device_header;
-	uint32* size = (uint32*)buffer;
+	device_header.vendor_id = device->vendor;
+	device_header.product_id = device->product;
+	device_header.max_packet_size = device->max_packet_size;
 
-	ids[0] = device->vendor;
+	if (!IS_USER_ADDRESS(buffer)) {
-	ids[1] = device->product;
+		memcpy(buffer, &device_header, sizeof(wacom_device_header));
-	size[1] = device->max_packet_size;
+		return B_OK;
+	}
+
+	return user_memcpy(buffer, &device_header, sizeof(wacom_device_header));
 }
 
 // device_read
@@ -532,11 +541,11 @@ device_read(void* cookie, off_t pos, void* buf, size_t* count)
 
 	if (ret >= B_OK) {
 		// what the client "reads" is decided depending on how much bytes are
-		// provided 8 bytes are needed to "read" vendor id, product id and max
+		// provided. "sizeof(wacom_device_header)" bytes are needed to "read"
-		// packet size in case the client wants to read more than 8 bytes, a usb
+		// vendor id, product id and max packet size in case the client wants to
-		// interupt transfer is scheduled, and an error report is returned as
+		// read more than "sizeof(wacom_device_header)" bytes, a usb interupt
-		// appropriate
+		// transfer is scheduled, and an error report is returned as appropriate
-		if (*count > 8) {
+		if (*count > sizeof(wacom_device_header)) {
 			// queue the interrupt transfer
 			ret = usb->queue_interrupt(device->pipe, device->data,
 				device->max_packet_size, device_interupt_callback, device);
@@ -557,9 +566,8 @@ device_read(void* cookie, off_t pos, void* buf, size_t* count)
 						DPRINTF_INFO((ID "device_read(%p) name = \"%s%d\" -> "
 							"B_TIMED_OUT\n", cookie, kBasePublishPath,
 							device->number));
-						*count = 8;
+						*count = sizeof(wacom_device_header);
-						read_header(device, buffer);
+						ret = read_header(device, buffer);
-						ret = B_OK;
 					} else {
 						// any other error trying to acquire the semaphore
 						*count = 0;
@@ -568,10 +576,19 @@ device_read(void* cookie, off_t pos, void* buf, size_t* count)
 					if (device->status == 0/*B_USBD_SUCCESS*/) {
 						DPRINTF_INFO((ID "interrupt transfer - success\n"));
 						// copy the data from the buffer
-						dataLength = min_c(device->length, *count - 8);
+						dataLength = min_c(device->length,
-						*count = dataLength + 8;
+							*count - sizeof(wacom_device_header));
-						read_header(device, buffer);
+						*count = dataLength + sizeof(wacom_device_header);
-						memcpy(buffer + 8, device->data, dataLength);
+						ret = read_header(device, buffer);
+						if (ret == B_OK) {
+							if (IS_USER_ADDRESS(buffer))
+								ret = user_memcpy(
+									buffer + sizeof(wacom_device_header),
+									device->data, dataLength);
+							else
+								memcpy(buffer + sizeof(wacom_device_header),
+									device->data, dataLength);
+						}
 					} else {
 						// an error happened during the interrupt transfer
 						*count = 0;
@@ -586,13 +603,12 @@ device_read(void* cookie, off_t pos, void* buf, size_t* count)
 					"interrupt: %" B_PRId32 "\n", cookie, kBasePublishPath,
 					device->number, ret);
 			}
-		} else if (*count == 8) {
+		} else if (*count == sizeof(wacom_device_header)) {
-			read_header(device, buffer);
+			ret = read_header(device, buffer);
-			ret = B_OK;
 		} else {
 			dprintf(ID "device_read(%p) name = \"%s%d\" -> buffer size must be "
-				"at least 8 bytes!\n", cookie, kBasePublishPath,
+				"at least the size of the wacom_device_header struct!\n",
-				device->number);
+				cookie, kBasePublishPath, device->number);
 			*count = 0;
 			ret = B_BAD_VALUE;
 		}