* [PATCH 1/2] fixes for adt7410
@ 2012-07-15 20:40 Hartmut Knaack
2012-07-15 20:59 ` Lars-Peter Clausen
0 siblings, 1 reply; 5+ messages in thread
From: Hartmut Knaack @ 2012-07-15 20:40 UTC (permalink / raw)
To: linux-iio; +Cc: jic23
This patch adds some checks prior to accessing device attribute lists in industrialio-event.c.
Signed-off-by: Harmut Knaack <knaack.h@gmx.de>
---
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index b49059d..fe68e90 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -425,11 +425,13 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
*attrcount_orig);
attrn = attrcount_orig;
/* Add all elements from the list. */
- list_for_each_entry(p,
- &indio_dev->event_interface->dev_attr_list,
- l)
- indio_dev->event_interface->group.attrs[attrn++] =
- &p->dev_attr.attr;
+ if (indio_dev->event_interface->dev_attr_list.next)
+ list_for_each_entry(p,
+ &indio_dev->event_interface->dev_attr_list,
+ l)
+ indio_dev->event_interface->group.attrs[attrn++] =
+ &p->dev_attr.attr;
+
indio_dev->groups[indio_dev->groupcounter++] =
&indio_dev->event_interface->group;
@@ -447,7 +449,9 @@ void iio_device_unregister_eventset(struct iio_dev *indio_dev)
{
if (indio_dev->event_interface == NULL)
return;
- __iio_remove_event_config_attrs(indio_dev);
- kfree(indio_dev->event_interface->group.attrs);
+ if (indio_dev->event_interface->dev_attr_list.next)
+ __iio_remove_event_config_attrs(indio_dev);
+ if (indio_dev->event_interface->group.attrs)
+ kfree(indio_dev->event_interface->group.attrs);
kfree(indio_dev->event_interface);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fixes for adt7410
2012-07-15 20:40 [PATCH 1/2] fixes for adt7410 Hartmut Knaack
@ 2012-07-15 20:59 ` Lars-Peter Clausen
2012-07-15 21:50 ` Hartmut Knaack
0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2012-07-15 20:59 UTC (permalink / raw)
To: Hartmut Knaack; +Cc: linux-iio, jic23
On 07/15/2012 10:40 PM, Hartmut Knaack wrote:
> This patch adds some checks prior to accessing device attribute lists in industrialio-event.c.
>
> Signed-off-by: Harmut Knaack <knaack.h@gmx.de>
> ---
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index b49059d..fe68e90 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -425,11 +425,13 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> *attrcount_orig);
> attrn = attrcount_orig;
> /* Add all elements from the list. */
> - list_for_each_entry(p,
> - &indio_dev->event_interface->dev_attr_list,
> - l)
> - indio_dev->event_interface->group.attrs[attrn++] =
> - &p->dev_attr.attr;
> + if (indio_dev->event_interface->dev_attr_list.next)
next should never be NULL. If it is this means that INIT_LIST_HEAD has not
been called on the list and the correct fix is to make sure that it is called.
> + list_for_each_entry(p,
> + &indio_dev->event_interface->dev_attr_list,
> + l)
> + indio_dev->event_interface->group.attrs[attrn++] =
> + &p->dev_attr.attr;
> +
> indio_dev->groups[indio_dev->groupcounter++] =
> &indio_dev->event_interface->group;
>
> @@ -447,7 +449,9 @@ void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> {
> if (indio_dev->event_interface == NULL)
> return;
> - __iio_remove_event_config_attrs(indio_dev);
> - kfree(indio_dev->event_interface->group.attrs);
> + if (indio_dev->event_interface->dev_attr_list.next)
> + __iio_remove_event_config_attrs(indio_dev);
Same here.
> + if (indio_dev->event_interface->group.attrs)
> + kfree(indio_dev->event_interface->group.attrs);
kfree does a check for NULL itself.
> kfree(indio_dev->event_interface);
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fixes for adt7410
2012-07-15 20:59 ` Lars-Peter Clausen
@ 2012-07-15 21:50 ` Hartmut Knaack
2012-07-16 10:00 ` Lars-Peter Clausen
0 siblings, 1 reply; 5+ messages in thread
From: Hartmut Knaack @ 2012-07-15 21:50 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio, jic23
Lars-Peter Clausen schrieb:
> On 07/15/2012 10:40 PM, Hartmut Knaack wrote:
>> This patch adds some checks prior to accessing device attribute lists in industrialio-event.c.
>>
>> Signed-off-by: Harmut Knaack <knaack.h@gmx.de>
>> ---
>> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
>> index b49059d..fe68e90 100644
>> --- a/drivers/iio/industrialio-event.c
>> +++ b/drivers/iio/industrialio-event.c
>> @@ -425,11 +425,13 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
>> *attrcount_orig);
>> attrn = attrcount_orig;
>> /* Add all elements from the list. */
>> - list_for_each_entry(p,
>> - &indio_dev->event_interface->dev_attr_list,
>> - l)
>> - indio_dev->event_interface->group.attrs[attrn++] =
>> - &p->dev_attr.attr;
>> + if (indio_dev->event_interface->dev_attr_list.next)
> next should never be NULL. If it is this means that INIT_LIST_HEAD has not
> been called on the list and the correct fix is to make sure that it is called.
Exactly. If the channels-attributes have not been set, INIT_LIST_HEAD (called through __iio_add_event_config_attrs) is not called. So, should we just stop instantiating the device if it does not have channel information?
>
>> + list_for_each_entry(p,
>> + &indio_dev->event_interface->dev_attr_list,
>> + l)
>> + indio_dev->event_interface->group.attrs[attrn++] =
>> + &p->dev_attr.attr;
>> +
>> indio_dev->groups[indio_dev->groupcounter++] =
>> &indio_dev->event_interface->group;
>>
>> @@ -447,7 +449,9 @@ void iio_device_unregister_eventset(struct iio_dev *indio_dev)
>> {
>> if (indio_dev->event_interface == NULL)
>> return;
>> - __iio_remove_event_config_attrs(indio_dev);
>> - kfree(indio_dev->event_interface->group.attrs);
>> + if (indio_dev->event_interface->dev_attr_list.next)
>> + __iio_remove_event_config_attrs(indio_dev);
> Same here.
>
>> + if (indio_dev->event_interface->group.attrs)
>> + kfree(indio_dev->event_interface->group.attrs);
> kfree does a check for NULL itself.
>
>> kfree(indio_dev->event_interface);
>> }
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fixes for adt7410
2012-07-15 21:50 ` Hartmut Knaack
@ 2012-07-16 10:00 ` Lars-Peter Clausen
2012-07-16 19:26 ` Hartmut Knaack
0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2012-07-16 10:00 UTC (permalink / raw)
To: Hartmut Knaack; +Cc: linux-iio, jic23
On 07/15/2012 11:50 PM, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb:
>> On 07/15/2012 10:40 PM, Hartmut Knaack wrote:
>>> This patch adds some checks prior to accessing device attribute lists in industrialio-event.c.
>>>
>>> Signed-off-by: Harmut Knaack <knaack.h@gmx.de>
>>> ---
>>> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
>>> index b49059d..fe68e90 100644
>>> --- a/drivers/iio/industrialio-event.c
>>> +++ b/drivers/iio/industrialio-event.c
>>> @@ -425,11 +425,13 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
>>> *attrcount_orig);
>>> attrn = attrcount_orig;
>>> /* Add all elements from the list. */
>>> - list_for_each_entry(p,
>>> - &indio_dev->event_interface->dev_attr_list,
>>> - l)
>>> - indio_dev->event_interface->group.attrs[attrn++] =
>>> - &p->dev_attr.attr;
>>> + if (indio_dev->event_interface->dev_attr_list.next)
>> next should never be NULL. If it is this means that INIT_LIST_HEAD has not
>> been called on the list and the correct fix is to make sure that it is called.
> Exactly. If the channels-attributes have not been set, INIT_LIST_HEAD (called through __iio_add_event_config_attrs) is not called. So, should we just stop instantiating the device if it does not have channel information?
The correct fix is to call INIT_LIST_HEAD right after the struct which
contains the list has been allocated. Sascha already sent a patch which does
this as well. See: http://www.spinics.net/lists/linux-iio/msg05946.html
- Lars
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fixes for adt7410
2012-07-16 10:00 ` Lars-Peter Clausen
@ 2012-07-16 19:26 ` Hartmut Knaack
0 siblings, 0 replies; 5+ messages in thread
From: Hartmut Knaack @ 2012-07-16 19:26 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio, jic23
Lars-Peter Clausen schrieb:
> On 07/15/2012 11:50 PM, Hartmut Knaack wrote:
>> Lars-Peter Clausen schrieb:
>>> On 07/15/2012 10:40 PM, Hartmut Knaack wrote:
>>>> This patch adds some checks prior to accessing device attribute lists in industrialio-event.c.
>>>>
>>>> Signed-off-by: Harmut Knaack <knaack.h@gmx.de>
>>>> ---
>>>> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
>>>> index b49059d..fe68e90 100644
>>>> --- a/drivers/iio/industrialio-event.c
>>>> +++ b/drivers/iio/industrialio-event.c
>>>> @@ -425,11 +425,13 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
>>>> *attrcount_orig);
>>>> attrn = attrcount_orig;
>>>> /* Add all elements from the list. */
>>>> - list_for_each_entry(p,
>>>> - &indio_dev->event_interface->dev_attr_list,
>>>> - l)
>>>> - indio_dev->event_interface->group.attrs[attrn++] =
>>>> - &p->dev_attr.attr;
>>>> + if (indio_dev->event_interface->dev_attr_list.next)
>>> next should never be NULL. If it is this means that INIT_LIST_HEAD has not
>>> been called on the list and the correct fix is to make sure that it is called.
>> Exactly. If the channels-attributes have not been set, INIT_LIST_HEAD (called through __iio_add_event_config_attrs) is not called. So, should we just stop instantiating the device if it does not have channel information?
> The correct fix is to call INIT_LIST_HEAD right after the struct which
> contains the list has been allocated. Sascha already sent a patch which does
> this as well. See: http://www.spinics.net/lists/linux-iio/msg05946.html
>
> - Lars
>
Alright, never mind then. I'm looking forward to see those changes in mainline kernel :-)
Hartmut
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-16 19:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-15 20:40 [PATCH 1/2] fixes for adt7410 Hartmut Knaack
2012-07-15 20:59 ` Lars-Peter Clausen
2012-07-15 21:50 ` Hartmut Knaack
2012-07-16 10:00 ` Lars-Peter Clausen
2012-07-16 19:26 ` Hartmut Knaack
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.