All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hid_parse failure
@ 2012-05-01 16:07 srinivas pandruvada
  2012-05-10 10:21 ` Jiri Kosina
  0 siblings, 1 reply; 6+ messages in thread
From: srinivas pandruvada @ 2012-05-01 16:07 UTC (permalink / raw)
  To: linux-input; +Cc: srinivas pandruvada

When logical maximum is 0xffffffff, the parser fails even if
logical minimum is equal to or greater than  0.
By HID specification, if both the Logical Minimum and Logical
Maximum extents are defined as positive values (0 or greater)
then the report field can be assumed to be an unsigned value.
Otherwise, all integer values are signed values represented
in 2’s complement format.
---
 drivers/hid/hid-core.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c757f10..b616f4b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -225,8 +225,16 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 		return -1;
 	}
 
-	if (parser->global.logical_maximum < parser->global.logical_minimum) {
-		dbg_hid("logical range invalid %d %d\n", parser->global.logical_minimum, parser->global.logical_maximum);
+	if ((parser->global.logical_minimum < 0 &&
+		parser->global.logical_maximum <
+			parser->global.logical_minimum) ||
+		(parser->global.logical_minimum >= 0 &&
+			(__u32)parser->global.logical_maximum <
+			(__u32)parser->global.logical_minimum)
+		) {
+		dbg_hid("logical range invalid 0x%x 0x%x\n",
+			parser->global.logical_minimum,
+				parser->global.logical_maximum);
 		return -1;
 	}
 
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] hid_parse failure
@ 2012-05-09  2:06 Srinivas Pandruvada
  0 siblings, 0 replies; 6+ messages in thread
From: Srinivas Pandruvada @ 2012-05-09  2:06 UTC (permalink / raw)
  To: jkosina, linux-input

Any comments on my patch for hid parsing failure (submitted on May 1st)?

Thanks,
Srinivas


When logical maximum is 0xffffffff, the parser fails even if
logical minimum is equal to or greater than  0.
By HID specification, if both the Logical Minimum and Logical
Maximum extents are defined as positive values (0 or greater)
then the report field can be assumed to be an unsigned value.
Otherwise, all integer values are signed values represented
in 2’s complement format.
---
  drivers/hid/hid-core.c |   12 ++++++++++--
  1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c757f10..b616f4b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -225,8 +225,16 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
  		return -1;
  	}

-	if (parser->global.logical_maximum<  parser->global.logical_minimum) {
-		dbg_hid("logical range invalid %d %d\n", parser->global.logical_minimum, parser->global.logical_maximum);
+	if ((parser->global.logical_minimum<  0&&
+		parser->global.logical_maximum<
+			parser->global.logical_minimum) ||
+		(parser->global.logical_minimum>= 0&&
+			(__u32)parser->global.logical_maximum<
+			(__u32)parser->global.logical_minimum)
+		) {
+		dbg_hid("logical range invalid 0x%x 0x%x\n",
+			parser->global.logical_minimum,
+				parser->global.logical_maximum);
  		return -1;
  	}

-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] hid_parse failure
  2012-05-01 16:07 srinivas pandruvada
@ 2012-05-10 10:21 ` Jiri Kosina
  2012-05-10 14:04   ` Pandruvada, Srinivas
  0 siblings, 1 reply; 6+ messages in thread
From: Jiri Kosina @ 2012-05-10 10:21 UTC (permalink / raw)
  To: srinivas pandruvada; +Cc: linux-input

On Tue, 1 May 2012, srinivas pandruvada wrote:

> When logical maximum is 0xffffffff, the parser fails even if
> logical minimum is equal to or greater than  0.
> By HID specification, if both the Logical Minimum and Logical
> Maximum extents are defined as positive values (0 or greater)
> then the report field can be assumed to be an unsigned value.
> Otherwise, all integer values are signed values represented
> in 2’s complement format.

Signed-off-by: line is missing, could you please fix that?

> ---
>  drivers/hid/hid-core.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index c757f10..b616f4b 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -225,8 +225,16 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
>  		return -1;
>  	}
>  
> -	if (parser->global.logical_maximum < parser->global.logical_minimum) {
> -		dbg_hid("logical range invalid %d %d\n", parser->global.logical_minimum, parser->global.logical_maximum);
> +	if ((parser->global.logical_minimum < 0 &&
> +		parser->global.logical_maximum <
> +			parser->global.logical_minimum) ||
> +		(parser->global.logical_minimum >= 0 &&
> +			(__u32)parser->global.logical_maximum <
> +			(__u32)parser->global.logical_minimum)
> +		) {
> +		dbg_hid("logical range invalid 0x%x 0x%x\n",
> +			parser->global.logical_minimum,
> +				parser->global.logical_maximum);
>  		return -1;
>  	}

The change looks correct per se, but I will have to think a little bit 
more whether other places of report parsing don't need similar adjustments 
as well to work properly in such scenarios.

How did you find this out? By code/spec inspection, or do you actually 
have the device that exposes this problem?

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] hid_parse failure
  2012-05-10 10:21 ` Jiri Kosina
@ 2012-05-10 14:04   ` Pandruvada, Srinivas
  0 siblings, 0 replies; 6+ messages in thread
From: Pandruvada, Srinivas @ 2012-05-10 14:04 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input@vger.kernel.org


I have a sensor hub device, which exposes this problem.  I will send patch again.
I will send the patch again.

Thanks,
Srinivas


-----Original Message-----
From: Jiri Kosina [mailto:jkosina@suse.cz] 
Sent: Thursday, May 10, 2012 3:21 AM
To: Pandruvada, Srinivas
Cc: linux-input@vger.kernel.org
Subject: Re: [PATCH] hid_parse failure

On Tue, 1 May 2012, srinivas pandruvada wrote:

> When logical maximum is 0xffffffff, the parser fails even if logical 
> minimum is equal to or greater than  0.
> By HID specification, if both the Logical Minimum and Logical Maximum 
> extents are defined as positive values (0 or greater) then the report 
> field can be assumed to be an unsigned value.
> Otherwise, all integer values are signed values represented in 2’s 
> complement format.

Signed-off-by: line is missing, could you please fix that?

> ---
>  drivers/hid/hid-core.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 
> c757f10..b616f4b 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -225,8 +225,16 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
>  		return -1;
>  	}
>  
> -	if (parser->global.logical_maximum < parser->global.logical_minimum) {
> -		dbg_hid("logical range invalid %d %d\n", parser->global.logical_minimum, parser->global.logical_maximum);
> +	if ((parser->global.logical_minimum < 0 &&
> +		parser->global.logical_maximum <
> +			parser->global.logical_minimum) ||
> +		(parser->global.logical_minimum >= 0 &&
> +			(__u32)parser->global.logical_maximum <
> +			(__u32)parser->global.logical_minimum)
> +		) {
> +		dbg_hid("logical range invalid 0x%x 0x%x\n",
> +			parser->global.logical_minimum,
> +				parser->global.logical_maximum);
>  		return -1;
>  	}

The change looks correct per se, but I will have to think a little bit more whether other places of report parsing don't need similar adjustments as well to work properly in such scenarios.

How did you find this out? By code/spec inspection, or do you actually have the device that exposes this problem?

Thanks,

--
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] hid_parse failure
@ 2012-05-10 23:45 srinivas pandruvada
  2012-05-14 13:04 ` Jiri Kosina
  0 siblings, 1 reply; 6+ messages in thread
From: srinivas pandruvada @ 2012-05-10 23:45 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, srinivas pandruvada

When logical maximum is 0xffffffff, the parser fails even if
logical minimum is more than 0.
By HID specification this is a valid combination.

Signed-off-by: srinivas pandruvada <srinivas.pandruvada@intel.com>
---
 drivers/hid/hid-core.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 4da66b4..b7d758c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -230,9 +230,15 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 		return -1;
 	}
 
-	if (parser->global.logical_maximum < parser->global.logical_minimum) {
-		hid_err(parser->device, "logical range invalid %d %d\n",
-				parser->global.logical_minimum, parser->global.logical_maximum);
+	if ((parser->global.logical_minimum < 0 &&
+		parser->global.logical_maximum <
+		parser->global.logical_minimum) ||
+		(parser->global.logical_minimum >= 0 &&
+		(__u32)parser->global.logical_maximum <
+		(__u32)parser->global.logical_minimum)) {
+		dbg_hid("logical range invalid 0x%x 0x%x\n",
+			parser->global.logical_minimum,
+			parser->global.logical_maximum);
 		return -1;
 	}
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] hid_parse failure
  2012-05-10 23:45 srinivas pandruvada
@ 2012-05-14 13:04 ` Jiri Kosina
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Kosina @ 2012-05-14 13:04 UTC (permalink / raw)
  To: srinivas pandruvada; +Cc: linux-input

On Thu, 10 May 2012, srinivas pandruvada wrote:

> When logical maximum is 0xffffffff, the parser fails even if
> logical minimum is more than 0.
> By HID specification this is a valid combination.
> 
> Signed-off-by: srinivas pandruvada <srinivas.pandruvada@intel.com>

I have made the subject line a little bit more verbose, added a bit of 
explanatory comment before the condition, and applied.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-14 13:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-09  2:06 [PATCH] hid_parse failure Srinivas Pandruvada
  -- strict thread matches above, loose matches on Subject: below --
2012-05-10 23:45 srinivas pandruvada
2012-05-14 13:04 ` Jiri Kosina
2012-05-01 16:07 srinivas pandruvada
2012-05-10 10:21 ` Jiri Kosina
2012-05-10 14:04   ` Pandruvada, Srinivas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.