[Xenomai] Analogy: subdev list not intitialized if driver private data is NULL

Andreas Glatz andi.glatz at gmail.com
Sat Apr 12 14:14:21 CEST 2014


On 11 Apr 2014, at 14:15, Philippe Gerum wrote:

> On 04/10/2014 10:58 AM, Andreas Glatz wrote:
>> Currently my analogy driver for a new DAQ board doesn't need the  
>> private
>> data struct of the driver, so I set the size to 0:
>>
>> /* Analogy driver structure */
>> static a4l_drv_t a4l_drv_pandadaq = {
>>     .owner = THIS_MODULE,
>>     .board_name = "analogy_pandadaq",
>>     .attach = dev_pandadaq_attach,
>>     .detach = dev_pandadaq_detach,
>>     .privdata_size = 0,
>> };
>>
>> However, due to a potential bug the kernel crashes with a NULL ptr
>> exception after linking analogy0 to my driver.
>>
>> If I apply this patch to the git head of xenomai-2.6 everything works
>> for me:
>>
>> diff --git a/ksrc/drivers/analogy/device.c b/ksrc/drivers/analogy/ 
>> device.c
>> index b93ae72..c4095a2 100644
>> --- a/ksrc/drivers/analogy/device.c
>> +++ b/ksrc/drivers/analogy/device.c
>> @@ -278,15 +278,14 @@ int a4l_assign_driver(a4l_cxt_t * cxt,
>>         a4l_dev_t *dev = a4l_get_dev(cxt);
>>
>>         dev->driver = drv;
>> +
>> +       INIT_LIST_HEAD(&dev->subdvsq);
>>
>>         if (drv->privdata_size == 0)
>>                 __a4l_dbg(1, core_dbg,
>>                           "a4l_assign_driver: warning! "
>>                           "the field priv will not be usable\n");
>>         else {
>> -
>> -               INIT_LIST_HEAD(&dev->subdvsq);
>> -
>>                 dev->priv = rtdm_malloc(drv->privdata_size);
>>                 if (dev->priv == NULL && drv->privdata_size != 0) {
>>                         __a4l_err("a4l_assign_driver: "
>>
>>
>
> Makes sense. This release routine may also attempt to free a NULL  
> pointer, which is fortunately caught by the underlying allocator,  
> but is definitely invalid. Finally, the open-coded iterator for  
> subdevices reinvents a wheel. Could you try this patch adding a few  
> fixups to yours? TIA,
>
> diff --git a/ksrc/drivers/analogy/device.c b/ksrc/drivers/analogy/ 
> device.c
> index b93ae72..60b2977 100644
> --- a/ksrc/drivers/analogy/device.c
> +++ b/ksrc/drivers/analogy/device.c
> @@ -278,17 +278,15 @@ int a4l_assign_driver(a4l_cxt_t * cxt,
> 	a4l_dev_t *dev = a4l_get_dev(cxt);
>
> 	dev->driver = drv;
> +	INIT_LIST_HEAD(&dev->subdvsq);
>
> 	if (drv->privdata_size == 0)
> 		__a4l_dbg(1, core_dbg,
> 			  "a4l_assign_driver: warning! "
> 			  "the field priv will not be usable\n");
> 	else {
> -
> -		INIT_LIST_HEAD(&dev->subdvsq);
> -
> 		dev->priv = rtdm_malloc(drv->privdata_size);
> -		if (dev->priv == NULL && drv->privdata_size != 0) {
> +		if (dev->priv == NULL) {
> 			__a4l_err("a4l_assign_driver: "
> 				  "call(alloc) failed\n");
> 			ret = -ENOMEM;
> @@ -325,27 +323,26 @@ out_assign_driver:
>
> int a4l_release_driver(a4l_cxt_t * cxt)
> {
> -	int ret = 0;
> 	a4l_dev_t *dev = a4l_get_dev(cxt);
> +	a4l_subd_t *subd, *tmp;
> +	int ret = 0;
>
> 	if ((ret = dev->driver->detach(dev)) != 0)
> 		goto out_release_driver;
>
> -	/* Decrease module's count
> -	   so as to allow module unloading */
> 	module_put(dev->driver->owner);
>
> 	/* In case, the driver developer did not free the subdevices */
> -	while (&dev->subdvsq != dev->subdvsq.next) {
> -		struct list_head *this = dev->subdvsq.next;
> -		a4l_subd_t *tmp = list_entry(this, a4l_subd_t, list);
> -
> -		list_del(this);
> -		rtdm_free(tmp);
> -	}
> +	if (!list_empty(&dev->subdvsq))
> +		list_for_each_entry_safe(subd, tmp, &dev->subdvsq, list) {
> +			list_del(&subd->list);
> +			rtdm_free(subd);
> +		}
>
> 	/* Free the private field */
> -	rtdm_free(dev->priv);
> +	if (dev->priv)
> +		rtdm_free(dev->priv);
> +
> 	dev->driver = NULL;
>
> out_release_driver:
>
>

Your patch compiles cleanly and works for me.

I tested:

 > analogy_config analogy0 analogy_pandadaq
 > [...]
 > analogy_config -r analogy0
 > rmmod analogy_pandadaq
 > insmod analogy_pandadaq
 > [... and the same from the start]

I didn't see any warnings in dmesg...

A.





More information about the Xenomai mailing list