Re: Stack usage

["Ronny V. Vindenes" <s864@ii.uib.no>]

tir, 22,.03.2005 kl. 12.27 +0100, skrev Takashi Iwai:
> At Tue, 22 Mar 2005 11:22:47 +0100,
> I wrote:
> > 
> > > It looks like snd_emu10k1_add_controls is not too hard to fix, but maybe
> > > I am missing something.
> > 
> > It'd be easy - just use kmalloc() for the temporary
> > emu10k1_fx8010_control_gpr_t instance.
> 
> The below is a quick fix for emu10k1 part.  Could you give a try?
> 
> 
> Takashi
> Index: alsa-kernel/pci/emu10k1/emufx.c
> ===================================================================
> RCS file: /home/iwai/cvs/alsa/alsa-kernel/pci/emu10k1/emufx.c,v
> retrieving revision 1.67
> diff -u -r1.67 emufx.c
> --- alsa-kernel/pci/emu10k1/emufx.c	21 Mar 2005 19:43:42 -0000	1.67
> +++ alsa-kernel/pci/emu10k1/emufx.c	22 Mar 2005 11:25:46 -0000
> @@ -634,7 +634,8 @@
>  	snd_ctl_elem_id_t __user *_id;
>  	snd_ctl_elem_id_t id;
>  	emu10k1_fx8010_control_gpr_t __user *_gctl;
> -	emu10k1_fx8010_control_gpr_t gctl;
> +	emu10k1_fx8010_control_gpr_t *gctl;
> +	int err;
>  	
>  	for (i = 0, _id = icode->gpr_del_controls;
>  	     i < icode->gpr_del_control_count; i++, _id++) {
> @@ -643,28 +644,41 @@
>  		if (snd_emu10k1_look_for_ctl(emu, &id) == NULL)
>  			return -ENOENT;
>  	}
> +	gctl = kmalloc(sizeof(*gctl), GFP_KERNEL);
> +	if (! gctl)
> +		return -ENOMEM;
> +	err = 0;
>  	for (i = 0, _gctl = icode->gpr_add_controls;
>  	     i < icode->gpr_add_control_count; i++, _gctl++) {
> -		if (copy_from_user(&gctl, _gctl, sizeof(gctl)))
> -			return -EFAULT;
> -		if (snd_emu10k1_look_for_ctl(emu, &gctl.id))
> +		if (copy_from_user(gctl, _gctl, sizeof(*gctl))) {
> +			err = -EFAULT;
> +			goto __error;
> +		}
> +		if (snd_emu10k1_look_for_ctl(emu, &gctl->id))
>  			continue;
>  		down_read(&emu->card->controls_rwsem);
> -		if (snd_ctl_find_id(emu->card, &gctl.id) != NULL) {
> +		if (snd_ctl_find_id(emu->card, &gctl->id) != NULL) {
>  			up_read(&emu->card->controls_rwsem);
> -			return -EEXIST;
> +			err = -EEXIST;
> +			goto __error;
>  		}
>  		up_read(&emu->card->controls_rwsem);
> -		if (gctl.id.iface != SNDRV_CTL_ELEM_IFACE_MIXER &&
> -		    gctl.id.iface != SNDRV_CTL_ELEM_IFACE_PCM)
> -			return -EINVAL;
> +		if (gctl->id.iface != SNDRV_CTL_ELEM_IFACE_MIXER &&
> +		    gctl->id.iface != SNDRV_CTL_ELEM_IFACE_PCM) {
> +			err = -EINVAL;
> +			goto __error;
> +		}
>  	}
>  	for (i = 0, _gctl = icode->gpr_list_controls;
>  	     i < icode->gpr_list_control_count; i++, _gctl++) {
>  	     	/* FIXME: we need to check the WRITE access */
> -		if (copy_from_user(&gctl, _gctl, sizeof(gctl)))
> -			return -EFAULT;
> +		if (copy_from_user(gctl, _gctl, sizeof(*gctl))) {
> +			err = -EFAULT;
> +			goto __error;
> +		}
>  	}
> + __error:
> +	kfree(gctl);
>  	return 0;

This should be return err; right?

-- 
Ronny V. Vindenes <s864@ii.uib.no>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click