Re: [PATCH 2/4] ALSA: control: delegate checking the length of data payload to each drivers

[Vinod Koul <vinod.koul@intel.com>]

On Tue, Aug 30, 2016 at 08:44:43AM +0900, Takashi Sakamoto wrote:
> In ioctl(2) for TLV feature of ALSA control interface, an argument is
> a pointer to a data of 'struct snd_ctl_tlv' type in user space.
> The first field of this structure is for numerical ID of control element
> (numid), the second is for the length of the rest in byte unit (length).
> The rest is data payload (tlv).
> 
> In current implementation, ALSA control core checks the length field
> so that the lest is bigger than sizeof(unsigned int) * 2. This is due to
> original design of this feature.
> 
> Originally, this feature is designed to allow each driver to handle
> threshold level information. The rest of array stores the information.
> The information is expected to be this formation:
>  - type: e.g. SNDRV_CTL_TLVT_CONTAINER.
>  - length: length of the rest
>  - the rest: information fields
> 
> Against the original design, this feature can also be used to operate I/O.
> In this case, the data field in 'struct snd_ctl_tlv' is not necessarily
> the formation. Therefore, the check in control core is unreasonable.
> Perhaps, there's a case to transfer just 1 byte by this feature.
> 
> This commit purges the check and delegate it to each drivers. Fortunately,
> each implementation in drivers already check the length of payload.

Shouldn't it be other way around, we want to core to check these type of
things and not duplicate over drivers...

> @@ -1438,8 +1438,6 @@ static int snd_ctl_tlv_ioctl(struct snd_ctl_file *file,
>  
>  	if (copy_from_user(&tlv, _tlv, sizeof(tlv)))
>  		return -EFAULT;
> -	if (tlv.length < sizeof(unsigned int) * 2)
> -		return -EINVAL;
>  	if (!tlv.numid)
>  		return -EINVAL;
>  	down_read(&card->controls_rwsem);

-- 
~Vinod