Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>
>>>>>> -#define test_claimed(owner) ((long) (owner) & 1)
>>>>>> -#define clear_claimed(owner) ((xnthread_t *) ((long) (owner) & ~1))
>>>>>> -#define set_claimed(owner, bit) \
>>>>>> - ((xnthread_t *) ((long) clear_claimed(owner) | !!(bit)))
>>>>>> +#define __CLAIMED_BIT XN_HANDLE_SPARE3
>>>>>> +
>>>>>> +#define test_claimed(owner) xnhandle_test_spare(owner,
>>>>>> __CLAIMED_BIT)
>>>>>> +#define clear_claimed(owner) xnhandle_mask_spare(owner)
>>>>>> +#define set_claimed(owner, bit) ({ \
>>>>>> + xnhandle_t __tmp = xnhandle_mask_spare(owner); \
>>>>>> + if (bit) \
>>>>>> + xnhandle_set_spare(__tmp, __CLAIMED_BIT); \
>>>>>> + __tmp; \
>>>>>> +})
>>>>> I liked doing this with no conditional.
>>>> You mean let the caller pass 'bit' as 0 or __CLAIMED_BIT (the latter
>>>> would require some renaming then, I guess)?
>>>>
>>>>>> + xnarch_atomic_set(mutex->owner,
>>>>>> + set_claimed(xnthread_handle(owner),
>>>>>> +
>>>>>> xnsynch_nsleepers(&mutex->synchbase)));
>>>>> Ok. I think you have spotted a bug here. This should be mutex->sleepers
>>>>> instead of xnsynch_nsleepers.
>>>> OK. Will you fix? I will rebase then.
>>>>
>>>>>> + /* Consistency check for owner handle - is the object a thread?
>>>>>> */
>>>>>> + if (unlikely(xnthread_handle(owner) != clear_claimed(ownerh))) {
>>>>>> + err = -EINVAL;
>>>>>> + goto error;
>>>>>> }
>>>>> What is this ?
>>>> This ensures that we are not dereferences some arbitrary registry object
>>>> due to a borken handle in the mutex variable: The the object registered
>>>> on this handle is an xnthread, we will find the very same handle value
>>>> at the well-known place in the object. Kind of magic for xnthread
>>>> objects (instead of adding a magic field to them).
>>> Yes, but the point is: how can the handle be borken ? It looks like a
>>> useless check to me.
>> This is user memory. Everything can be borken. And I don't want to
>> create yet another source for user-triggered kernel bugs (including
>> silent ones) like with our heap management structures that are
>> corruptible by userland. That's headache guarantee.
>
> No, we assume that the user application will only clobber our heap if it
> is broken too. Add this with a XENO_DEBUG option if you want (not
> XENO_DEBUG(POSIX)).
No problem for me. This check is almost 100% identical (specifically
overhead-wise) to the usual object magic checks after querying the
registry, and as those are unconditionally performed, I saw no need to
handle this case differently.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
