[alsa-devel] [PATCH] ALSA: asihpi - Clarify adapter index validity check.
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));
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;
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
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"
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
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));
participants (3)
-
Eliot Blennerhassett -
linux＠audioscience.com
-
Takashi Iwai