All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: asihpi - Clarify adapter index validity check.
@ 2011-07-27 21:50 linux
  2011-07-28  5:43 ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: linux @ 2011-07-27 21:50 UTC (permalink / raw)
  To: patch; +Cc: tiwai, Eliot Blennerhassett, alsa-devel

From: Eliot Blennerhassett <eblennerhassett@audioscience.com>

Avoids assigning possibly invalid address to pa, even if it
is never dereferenced.

Signed-off-by: Eliot Blennerhassett <eblennerhassett@audioscience.com>
---
 sound/pci/asihpi/hpioctl.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
index 9683f84..a32502e 100644
--- a/sound/pci/asihpi/hpioctl.c
+++ b/sound/pci/asihpi/hpioctl.c
@@ -177,16 +177,21 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	} else {
 		u16 __user *ptr = NULL;
 		u32 size = 0;
-
+		u32 adapter_present;
 		/* -1=no data 0=read from user mem, 1=write to user mem */
 		int wrflag = -1;
-		u32 adapter = hm->h.adapter_index;
-		struct hpi_adapter *pa = &adapters[adapter];
+		struct hpi_adapter *pa;
+
+		if (hm->h.adapter_index < HPI_MAX_ADAPTERS) {
+			pa = &adapters[hm->h.adapter_index];
+			adapter_present = pa->type;
+		} else {
+			adapter_present = 0;
+		}
 
-		if ((adapter >= HPI_MAX_ADAPTERS) || (!pa->type)) {
-			hpi_init_response(&hr->r0, HPI_OBJ_ADAPTER,
-				HPI_ADAPTER_OPEN,
-				HPI_ERROR_BAD_ADAPTER_NUMBER);
+		if (!adapter_present) {
+			hpi_init_response(&hr->r0, hm->h.object,
+				hm->h.function, HPI_ERROR_BAD_ADAPTER_NUMBER);
 
 			uncopied_bytes =
 				copy_to_user(puhr, hr, sizeof(hr->h));
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: asihpi - Clarify adapter index validity check.
  2011-07-27 21:50 [PATCH] ALSA: asihpi - Clarify adapter index validity check linux
@ 2011-07-28  5:43 ` Takashi Iwai
  2011-07-28 22:14   ` Eliot Blennerhassett
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2011-07-28  5:43 UTC (permalink / raw)
  To: linux; +Cc: Eliot Blennerhassett, alsa-devel

At Thu, 28 Jul 2011 09:50:17 +1200,
linux@audioscience.com wrote:
> 
> From: Eliot Blennerhassett <eblennerhassett@audioscience.com>
> 
> Avoids assigning possibly invalid address to pa, even if it
> is never dereferenced.
> 
> Signed-off-by: Eliot Blennerhassett <eblennerhassett@audioscience.com>
> ---
>  sound/pci/asihpi/hpioctl.c |   19 ++++++++++++-------
>  1 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
> index 9683f84..a32502e 100644
> --- a/sound/pci/asihpi/hpioctl.c
> +++ b/sound/pci/asihpi/hpioctl.c
> @@ -177,16 +177,21 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	} else {
>  		u16 __user *ptr = NULL;
>  		u32 size = 0;
> -
> +		u32 adapter_present;
>  		/* -1=no data 0=read from user mem, 1=write to user mem */
>  		int wrflag = -1;
> -		u32 adapter = hm->h.adapter_index;
> -		struct hpi_adapter *pa = &adapters[adapter];
> +		struct hpi_adapter *pa;
> +
> +		if (hm->h.adapter_index < HPI_MAX_ADAPTERS) {
> +			pa = &adapters[hm->h.adapter_index];
> +			adapter_present = pa->type;
> +		} else {
> +			adapter_present = 0;
> +		}
>  
> -		if ((adapter >= HPI_MAX_ADAPTERS) || (!pa->type)) {
> -			hpi_init_response(&hr->r0, HPI_OBJ_ADAPTER,
> -				HPI_ADAPTER_OPEN,
> -				HPI_ERROR_BAD_ADAPTER_NUMBER);
> +		if (!adapter_present) {
> +			hpi_init_response(&hr->r0, hm->h.object,
> +				hm->h.function, HPI_ERROR_BAD_ADAPTER_NUMBER);

Here you are initializing to different values (from HPI_OBJ_ADAPTER to
hm->h.object, etc).  Is it intentional?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: asihpi - Clarify adapter index validity check.
  2011-07-28  5:43 ` Takashi Iwai
@ 2011-07-28 22:14   ` Eliot Blennerhassett
  2011-07-29  5:30     ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Eliot Blennerhassett @ 2011-07-28 22:14 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 28/07/11 17:43, Takashi Iwai wrote:
		if ((adapter >= HPI_MAX_ADAPTERS) || (!pa->type)) {
>> -			hpi_init_response(&hr->r0, HPI_OBJ_ADAPTER,
>> -				HPI_ADAPTER_OPEN,
>> -				HPI_ERROR_BAD_ADAPTER_NUMBER);
>> +		if (!adapter_present) {
>> +			hpi_init_response(&hr->r0, hm->h.object,
>> +				hm->h.function, HPI_ERROR_BAD_ADAPTER_NUMBER);
> 
> Here you are initializing to different values (from HPI_OBJ_ADAPTER to
> hm->h.object, etc).  Is it intentional?

Yes it is. It is so the error response reflects the parameters of the
corresponding message.   It makes for better error logging in userspace.

I can redo the patch with better commit log if you like.
Adding "Correct error response to reflect message object/function ids"


-- 
Eliot Blennerhassett
AudioScience Inc.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: asihpi - Clarify adapter index validity check.
  2011-07-28 22:14   ` Eliot Blennerhassett
@ 2011-07-29  5:30     ` Takashi Iwai
  2011-08-01 21:44       ` linux
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2011-07-29  5:30 UTC (permalink / raw)
  To: Eliot Blennerhassett; +Cc: alsa-devel

At Fri, 29 Jul 2011 10:14:26 +1200,
Eliot Blennerhassett wrote:
> 
> On 28/07/11 17:43, Takashi Iwai wrote:
> 		if ((adapter >= HPI_MAX_ADAPTERS) || (!pa->type)) {
> >> -			hpi_init_response(&hr->r0, HPI_OBJ_ADAPTER,
> >> -				HPI_ADAPTER_OPEN,
> >> -				HPI_ERROR_BAD_ADAPTER_NUMBER);
> >> +		if (!adapter_present) {
> >> +			hpi_init_response(&hr->r0, hm->h.object,
> >> +				hm->h.function, HPI_ERROR_BAD_ADAPTER_NUMBER);
> > 
> > Here you are initializing to different values (from HPI_OBJ_ADAPTER to
> > hm->h.object, etc).  Is it intentional?
> 
> Yes it is. It is so the error response reflects the parameters of the
> corresponding message.   It makes for better error logging in userspace.
> 
> I can redo the patch with better commit log if you like.
> Adding "Correct error response to reflect message object/function ids"

Yes, please give a matching changelog, then.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] ALSA: asihpi - Clarify adapter index validity check
  2011-07-29  5:30     ` Takashi Iwai
@ 2011-08-01 21:44       ` linux
  0 siblings, 0 replies; 5+ messages in thread
From: linux @ 2011-08-01 21:44 UTC (permalink / raw)
  To: patch; +Cc: tiwai, Eliot Blennerhassett, alsa-devel

From: Eliot Blennerhassett <eblennerhassett@audioscience.com>

Avoids assigning possibly invalid address to pa, even if it
is never dereferenced.
Correct error response to reflect request object/function ids.

Signed-off-by: Eliot Blennerhassett <eblennerhassett@audioscience.com>
---
 sound/pci/asihpi/hpioctl.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c
index 9683f84..a32502e 100644
--- a/sound/pci/asihpi/hpioctl.c
+++ b/sound/pci/asihpi/hpioctl.c
@@ -177,16 +177,21 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	} else {
 		u16 __user *ptr = NULL;
 		u32 size = 0;
-
+		u32 adapter_present;
 		/* -1=no data 0=read from user mem, 1=write to user mem */
 		int wrflag = -1;
-		u32 adapter = hm->h.adapter_index;
-		struct hpi_adapter *pa = &adapters[adapter];
+		struct hpi_adapter *pa;
+
+		if (hm->h.adapter_index < HPI_MAX_ADAPTERS) {
+			pa = &adapters[hm->h.adapter_index];
+			adapter_present = pa->type;
+		} else {
+			adapter_present = 0;
+		}
 
-		if ((adapter >= HPI_MAX_ADAPTERS) || (!pa->type)) {
-			hpi_init_response(&hr->r0, HPI_OBJ_ADAPTER,
-				HPI_ADAPTER_OPEN,
-				HPI_ERROR_BAD_ADAPTER_NUMBER);
+		if (!adapter_present) {
+			hpi_init_response(&hr->r0, hm->h.object,
+				hm->h.function, HPI_ERROR_BAD_ADAPTER_NUMBER);
 
 			uncopied_bytes =
 				copy_to_user(puhr, hr, sizeof(hr->h));
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-08-01 21:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-27 21:50 [PATCH] ALSA: asihpi - Clarify adapter index validity check linux
2011-07-28  5:43 ` Takashi Iwai
2011-07-28 22:14   ` Eliot Blennerhassett
2011-07-29  5:30     ` Takashi Iwai
2011-08-01 21:44       ` linux

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.