| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1414432614.18896.1.camel@perches.com>
Date: Mon, 27 Oct 2014 10:56:54 -0700
From: Joe Perches <joe@...ches.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [PATCH 03/11] aiptek: Fix probable mask then right shift defects
On Mon, 2014-10-27 at 07:44 -0700, Dmitry Torokhov wrote:
> Hi Joe,
Hello Dmitry.
> On Sun, Oct 26, 2014 at 10:24:59PM -0700, Joe Perches wrote:
> > Precedence of & and >> is not the same and is not left to right.
> > shift has higher precedence and should be done after the mask.
>
> Looking at the protocol description the current code is exactly right.
> We want to "move" button bits first as in packet type 1 they are in a
> different place than in other packets.
>
> I'll take a patch that adds parenthesis around shifts to make clear it
> is intended.
I think it's more sensible to do the shift first to a
temporary then direct comparisons.
Perhaps something like this cleanup that also uses
a temporary for aiptek->curSetting and
!!(foo & mask) instead of ((foo & mask) != 0) ? 1 : 0;
---
drivers/input/tablet/aiptek.c | 149 ++++++++++++++++++++----------------------
1 file changed, 72 insertions(+), 77 deletions(-)
diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index e7f966d..9c46618 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -433,6 +433,7 @@ static const char *map_val_to_str(const struct aiptek_map *map, int val)
static void aiptek_irq(struct urb *urb)
{
struct aiptek *aiptek = urb->context;
+ struct aiptek_settings *settings = &aiptek->curSetting;
unsigned char *data = aiptek->data;
struct input_dev *inputdev = aiptek->inputdev;
struct usb_interface *intf = aiptek->intf;
@@ -472,26 +473,31 @@ static void aiptek_irq(struct urb *urb)
* tool generated the event.
*/
if (data[0] == 1) {
- if (aiptek->curSetting.coordinateMode ==
+ if (settings->coordinateMode ==
AIPTEK_COORDINATE_ABSOLUTE_MODE) {
aiptek->diagnostic =
AIPTEK_DIAGNOSTIC_SENDING_RELATIVE_IN_ABSOLUTE;
} else {
- x = (signed char) data[2];
- y = (signed char) data[3];
-
- /* jitterable keeps track of whether any button has been pressed.
- * We're also using it to remap the physical mouse button mask
- * to pseudo-settings. (We don't specifically care about it's
- * value after moving/transposing mouse button bitmasks, except
+ /*
+ * Shift buttons pressed to the curSettings location.
+ * jitterable keeps track of whether any button has
+ * been pressed. We're also using it to remap the
+ * physical mouse button mask to pseudo-settings.
+ * (We don't specifically care about it's value after
+ * moving/transposing mouse button bitmasks, except
* that a non-zero value indicates that one or more
* mouse button was pressed.)
*/
+ unsigned char buttons = data[1] << 2;
+
+ x = (signed char)data[2];
+ y = (signed char)data[3];
+
jitterable = data[1] & 0x07;
- left = (data[1] & aiptek->curSetting.mouseButtonLeft >> 2) != 0 ? 1 : 0;
- right = (data[1] & aiptek->curSetting.mouseButtonRight >> 2) != 0 ? 1 : 0;
- middle = (data[1] & aiptek->curSetting.mouseButtonMiddle >> 2) != 0 ? 1 : 0;
+ left = !!(buttons & settings->mouseButtonLeft);
+ right = !!(buttons & settings->mouseButtonRight);
+ middle = !!(buttons & settings->mouseButtonMiddle);
input_report_key(inputdev, BTN_LEFT, left);
input_report_key(inputdev, BTN_MIDDLE, middle);
@@ -505,10 +511,10 @@ static void aiptek_irq(struct urb *urb)
/* Wheel support is in the form of a single-event
* firing.
*/
- if (aiptek->curSetting.wheel != AIPTEK_WHEEL_DISABLE) {
+ if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
input_report_rel(inputdev, REL_WHEEL,
- aiptek->curSetting.wheel);
- aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+ settings->wheel);
+ settings->wheel = AIPTEK_WHEEL_DISABLE;
}
if (aiptek->lastMacro != -1) {
input_report_key(inputdev,
@@ -522,26 +528,26 @@ static void aiptek_irq(struct urb *urb)
* absolute coordinates.
*/
else if (data[0] == 2) {
- if (aiptek->curSetting.coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
+ if (settings->coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
aiptek->diagnostic = AIPTEK_DIAGNOSTIC_SENDING_ABSOLUTE_IN_RELATIVE;
} else if (!AIPTEK_POINTER_ALLOW_STYLUS_MODE
- (aiptek->curSetting.pointerMode)) {
+ (settings->pointerMode)) {
aiptek->diagnostic = AIPTEK_DIAGNOSTIC_TOOL_DISALLOWED;
} else {
x = get_unaligned_le16(data + 1);
y = get_unaligned_le16(data + 3);
z = get_unaligned_le16(data + 6);
- dv = (data[5] & 0x01) != 0 ? 1 : 0;
- p = (data[5] & 0x02) != 0 ? 1 : 0;
- tip = (data[5] & 0x04) != 0 ? 1 : 0;
+ dv = !!(data[5] & 0x01);
+ p = !!(data[5] & 0x02);
+ tip = !!(data[5] & 0x04);
/* Use jitterable to re-arrange button masks
*/
jitterable = data[5] & 0x18;
- bs = (data[5] & aiptek->curSetting.stylusButtonLower) != 0 ? 1 : 0;
- pck = (data[5] & aiptek->curSetting.stylusButtonUpper) != 0 ? 1 : 0;
+ bs = !!(data[5] & settings->stylusButtonLower);
+ pck = !!(data[5] & settings->stylusButtonUpper);
/* dv indicates 'data valid' (e.g., the tablet is in sync
* and has delivered a "correct" report) We will ignore
@@ -552,14 +558,14 @@ static void aiptek_irq(struct urb *urb)
* tool key, and set the new one.
*/
if (aiptek->previousToolMode !=
- aiptek->curSetting.toolMode) {
+ settings->toolMode) {
input_report_key(inputdev,
aiptek->previousToolMode, 0);
input_report_key(inputdev,
- aiptek->curSetting.toolMode,
+ settings->toolMode,
1);
aiptek->previousToolMode =
- aiptek->curSetting.toolMode;
+ settings->toolMode;
}
if (p != 0) {
@@ -571,27 +577,25 @@ static void aiptek_irq(struct urb *urb)
input_report_key(inputdev, BTN_STYLUS, bs);
input_report_key(inputdev, BTN_STYLUS2, pck);
- if (aiptek->curSetting.xTilt !=
- AIPTEK_TILT_DISABLE) {
+ if (settings->xTilt != AIPTEK_TILT_DISABLE) {
input_report_abs(inputdev,
ABS_TILT_X,
- aiptek->curSetting.xTilt);
+ settings->xTilt);
}
- if (aiptek->curSetting.yTilt != AIPTEK_TILT_DISABLE) {
+ if (settings->yTilt != AIPTEK_TILT_DISABLE) {
input_report_abs(inputdev,
ABS_TILT_Y,
- aiptek->curSetting.yTilt);
+ settings->yTilt);
}
/* Wheel support is in the form of a single-event
* firing.
*/
- if (aiptek->curSetting.wheel !=
- AIPTEK_WHEEL_DISABLE) {
+ if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
input_report_abs(inputdev,
ABS_WHEEL,
- aiptek->curSetting.wheel);
- aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+ settings->wheel);
+ settings->wheel = AIPTEK_WHEEL_DISABLE;
}
}
input_report_abs(inputdev, ABS_MISC, p | AIPTEK_REPORT_TOOL_STYLUS);
@@ -607,10 +611,10 @@ static void aiptek_irq(struct urb *urb)
/* Report 3's come from the mouse in absolute mode.
*/
else if (data[0] == 3) {
- if (aiptek->curSetting.coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
+ if (settings->coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
aiptek->diagnostic = AIPTEK_DIAGNOSTIC_SENDING_ABSOLUTE_IN_RELATIVE;
} else if (!AIPTEK_POINTER_ALLOW_MOUSE_MODE
- (aiptek->curSetting.pointerMode)) {
+ (settings->pointerMode)) {
aiptek->diagnostic = AIPTEK_DIAGNOSTIC_TOOL_DISALLOWED;
} else {
x = get_unaligned_le16(data + 1);
@@ -618,25 +622,25 @@ static void aiptek_irq(struct urb *urb)
jitterable = data[5] & 0x1c;
- dv = (data[5] & 0x01) != 0 ? 1 : 0;
- p = (data[5] & 0x02) != 0 ? 1 : 0;
- left = (data[5] & aiptek->curSetting.mouseButtonLeft) != 0 ? 1 : 0;
- right = (data[5] & aiptek->curSetting.mouseButtonRight) != 0 ? 1 : 0;
- middle = (data[5] & aiptek->curSetting.mouseButtonMiddle) != 0 ? 1 : 0;
+ dv = !!(data[5] & 0x01);
+ p = !!(data[5] & 0x02);
+ left = !!(data[5] & settings->mouseButtonLeft);
+ right = !!(data[5] & settings->mouseButtonRight);
+ middle = !!(data[5] & settings->mouseButtonMiddle);
if (dv != 0) {
/* If the selected tool changed, reset the old
* tool key, and set the new one.
*/
if (aiptek->previousToolMode !=
- aiptek->curSetting.toolMode) {
+ settings->toolMode) {
input_report_key(inputdev,
aiptek->previousToolMode, 0);
input_report_key(inputdev,
- aiptek->curSetting.toolMode,
+ settings->toolMode,
1);
aiptek->previousToolMode =
- aiptek->curSetting.toolMode;
+ settings->toolMode;
}
if (p != 0) {
@@ -650,11 +654,11 @@ static void aiptek_irq(struct urb *urb)
/* Wheel support is in the form of a single-event
* firing.
*/
- if (aiptek->curSetting.wheel != AIPTEK_WHEEL_DISABLE) {
+ if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
input_report_abs(inputdev,
ABS_WHEEL,
- aiptek->curSetting.wheel);
- aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+ settings->wheel);
+ settings->wheel = AIPTEK_WHEEL_DISABLE;
}
}
input_report_abs(inputdev, ABS_MISC, p | AIPTEK_REPORT_TOOL_MOUSE);
@@ -672,11 +676,11 @@ static void aiptek_irq(struct urb *urb)
else if (data[0] == 4) {
jitterable = data[1] & 0x18;
- dv = (data[1] & 0x01) != 0 ? 1 : 0;
- p = (data[1] & 0x02) != 0 ? 1 : 0;
- tip = (data[1] & 0x04) != 0 ? 1 : 0;
- bs = (data[1] & aiptek->curSetting.stylusButtonLower) != 0 ? 1 : 0;
- pck = (data[1] & aiptek->curSetting.stylusButtonUpper) != 0 ? 1 : 0;
+ dv = !!(data[1] & 0x01);
+ p = !!(data[1] & 0x02);
+ tip = !!(data[1] & 0x04);
+ bs = !!(data[1] & settings->stylusButtonLower);
+ pck = !!(data[1] & settings->stylusButtonUpper);
macro = dv && p && tip && !(data[3] & 1) ? (data[3] >> 1) : -1;
z = get_unaligned_le16(data + 4);
@@ -685,15 +689,12 @@ static void aiptek_irq(struct urb *urb)
/* If the selected tool changed, reset the old
* tool key, and set the new one.
*/
- if (aiptek->previousToolMode !=
- aiptek->curSetting.toolMode) {
+ if (aiptek->previousToolMode != settings->toolMode) {
input_report_key(inputdev,
aiptek->previousToolMode, 0);
input_report_key(inputdev,
- aiptek->curSetting.toolMode,
- 1);
- aiptek->previousToolMode =
- aiptek->curSetting.toolMode;
+ settings->toolMode, 1);
+ aiptek->previousToolMode = settings->toolMode;
}
}
@@ -715,24 +716,23 @@ static void aiptek_irq(struct urb *urb)
else if (data[0] == 5) {
jitterable = data[1] & 0x1c;
- dv = (data[1] & 0x01) != 0 ? 1 : 0;
- p = (data[1] & 0x02) != 0 ? 1 : 0;
- left = (data[1]& aiptek->curSetting.mouseButtonLeft) != 0 ? 1 : 0;
- right = (data[1] & aiptek->curSetting.mouseButtonRight) != 0 ? 1 : 0;
- middle = (data[1] & aiptek->curSetting.mouseButtonMiddle) != 0 ? 1 : 0;
+ dv = !!(data[1] & 0x01);
+ p = !!(data[1] & 0x02);
+ left = !!(data[1]& settings->mouseButtonLeft);
+ right = !!(data[1] & settings->mouseButtonRight);
+ middle = !!(data[1] & settings->mouseButtonMiddle);
macro = dv && p && left && !(data[3] & 1) ? (data[3] >> 1) : 0;
if (dv) {
/* If the selected tool changed, reset the old
* tool key, and set the new one.
*/
- if (aiptek->previousToolMode !=
- aiptek->curSetting.toolMode) {
+ if (aiptek->previousToolMode != settings->toolMode) {
input_report_key(inputdev,
aiptek->previousToolMode, 0);
input_report_key(inputdev,
- aiptek->curSetting.toolMode, 1);
- aiptek->previousToolMode = aiptek->curSetting.toolMode;
+ settings->toolMode, 1);
+ aiptek->previousToolMode = settings->toolMode;
}
}
@@ -770,15 +770,10 @@ static void aiptek_irq(struct urb *urb)
/* If the selected tool changed, reset the old
tool key, and set the new one.
*/
- if (aiptek->previousToolMode !=
- aiptek->curSetting.toolMode) {
- input_report_key(inputdev,
- aiptek->previousToolMode, 0);
- input_report_key(inputdev,
- aiptek->curSetting.toolMode,
- 1);
- aiptek->previousToolMode =
- aiptek->curSetting.toolMode;
+ if (aiptek->previousToolMode != settings->toolMode) {
+ input_report_key(inputdev, aiptek->previousToolMode, 0);
+ input_report_key(inputdev, settings->toolMode, 1);
+ aiptek->previousToolMode = settings->toolMode;
}
input_report_key(inputdev, macroKeyEvents[macro], 1);
@@ -802,9 +797,9 @@ static void aiptek_irq(struct urb *urb)
*/
if (aiptek->previousJitterable != jitterable &&
- aiptek->curSetting.jitterDelay != 0 && aiptek->inDelay != 1) {
+ settings->jitterDelay != 0 && aiptek->inDelay != 1) {
aiptek->endDelay = jiffies +
- ((aiptek->curSetting.jitterDelay * HZ) / 1000);
+ ((settings->jitterDelay * HZ) / 1000);
aiptek->inDelay = 1;
}
aiptek->previousJitterable = jitterable;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists