From 29ae0e0f61022017dc3af1c46a59bc8a7259b83b Mon Sep 17 00:00:00 2001 From: CodeforEvolution Date: Fri, 22 May 2020 22:36:08 -0500 Subject: [PATCH] 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 --- headers/private/input/wacom_driver.h | 19 ++++++ .../devices/wacom/DeviceReader.cpp | 47 ++++++------- .../input_server/devices/wacom/DeviceReader.h | 2 +- .../kernel/drivers/input/wacom/Jamfile | 2 + .../kernel/drivers/input/wacom/wacom.c | 66 ++++++++++++------- 5 files changed, 88 insertions(+), 48 deletions(-) create mode 100644 headers/private/input/wacom_driver.h diff --git a/headers/private/input/wacom_driver.h b/headers/private/input/wacom_driver.h new file mode 100644 index 0000000000..e5101c9218 --- /dev/null +++ b/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 + + +typedef struct { + uint16 vendor_id; + uint16 product_id; + size_t max_packet_size; +} _PACKED wacom_device_header; + + +#endif // WACOM_DRIVER_H diff --git a/src/add-ons/input_server/devices/wacom/DeviceReader.cpp b/src/add-ons/input_server/devices/wacom/DeviceReader.cpp index 9b751f20e2..d8c5ba90b0 100644 --- a/src/add-ons/input_server/devices/wacom/DeviceReader.cpp +++ b/src/add-ons/input_server/devices/wacom/DeviceReader.cpp @@ -1,7 +1,10 @@ /* - * Copyright 2005-2008 Stephan Aßmus . All rights reserved. - * Distributed under the terms of the MIT license. + * Copyright 2005-2008 Stephan Aßmus + * Copyright 2020 Jacob Secunda + * All rights reserved. Distributed under the terms of the MIT license. */ + + #include "DeviceReader.h" #include @@ -9,19 +12,21 @@ #include +#include + #include "MasterServerDevice.h" -static ssize_t kHeaderSize = 8; - +static ssize_t kHeaderSize = sizeof(wacom_device_header); // constructor DeviceReader::DeviceReader() - : fDevicePath(NULL), - fDeviceFile(NULL), - fVendorID(0), - fProductID(0), - fMaxPackedSize(0) + : + fDevicePath(NULL), + fDeviceFile(NULL), + fVendorID(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]; - ret = fDeviceFile->Read(buffer, kHeaderSize); + wacom_device_header device_header; + ret = fDeviceFile->Read(&device_header, kHeaderSize); if (ret == kHeaderSize) { ret = B_OK; - uint16* ids = (uint16*)buffer; - fVendorID = ids[0]; - fProductID = ids[1]; - uint32* ps = (uint32*)buffer; - fMaxPackedSize = ps[1]; + fVendorID = device_header.vendor_id; + fProductID = device_header.product_id; + fMaxPacketSize = device_header.max_packet_size; } 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; } diff --git a/src/add-ons/input_server/devices/wacom/DeviceReader.h b/src/add-ons/input_server/devices/wacom/DeviceReader.h index f0b549e4f8..c8c1d8a36a 100644 --- a/src/add-ons/input_server/devices/wacom/DeviceReader.h +++ b/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 diff --git a/src/add-ons/kernel/drivers/input/wacom/Jamfile b/src/add-ons/kernel/drivers/input/wacom/Jamfile index 585ab2b937..8a8ef7b1c0 100644 --- a/src/add-ons/kernel/drivers/input/wacom/Jamfile +++ b/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 diff --git a/src/add-ons/kernel/drivers/input/wacom/wacom.c b/src/add-ons/kernel/drivers/input/wacom/wacom.c index f2bacc908f..773edcebc0 100644 --- a/src/add-ons/kernel/drivers/input/wacom/wacom.c +++ b/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 #include +#include +#include + 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; - uint32* size = (uint32*)buffer; + wacom_device_header device_header; + 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; - ids[1] = device->product; - size[1] = device->max_packet_size; + if (!IS_USER_ADDRESS(buffer)) { + memcpy(buffer, &device_header, sizeof(wacom_device_header)); + 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 - // packet size in case the client wants to read more than 8 bytes, a usb - // interupt transfer is scheduled, and an error report is returned as - // appropriate - if (*count > 8) { + // provided. "sizeof(wacom_device_header)" bytes are needed to "read" + // vendor id, product id and max packet size in case the client wants to + // read more than "sizeof(wacom_device_header)" bytes, a usb interupt + // transfer is scheduled, and an error report is returned as appropriate + 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; - read_header(device, buffer); - ret = B_OK; + *count = sizeof(wacom_device_header); + ret = read_header(device, buffer); } 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); - *count = dataLength + 8; - read_header(device, buffer); - memcpy(buffer + 8, device->data, dataLength); + dataLength = min_c(device->length, + *count - sizeof(wacom_device_header)); + *count = dataLength + sizeof(wacom_device_header); + 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) { - read_header(device, buffer); - ret = B_OK; + } else if (*count == sizeof(wacom_device_header)) { + ret = read_header(device, buffer); } else { dprintf(ID "device_read(%p) name = \"%s%d\" -> buffer size must be " - "at least 8 bytes!\n", cookie, kBasePublishPath, - device->number); + "at least the size of the wacom_device_header struct!\n", + cookie, kBasePublishPath, device->number); *count = 0; ret = B_BAD_VALUE; }