Re: Stack usage

[Takashi Iwai <tiwai@suse.de>]

At Tue, 22 Mar 2005 12:38:17 +0100,
Ronny V. Vindenes wrote:
> 
> 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?

Yep, thanks.


Takashi


-------------------------------------------------------
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