Marc Kleine-Budde wrote:
> Oliver Hartkopp wrote:
>> Marc Kleine-Budde wrote:
>>> Oliver Hartkopp wrote:
>>>>>> + *cf = (struct can_frame *)skb_put(skb, sizeof(struct
>>>>>> can_frame));
>>>> This could be also the place, where we could memset the struct can_frame to
>>>> zero, right?
>>>> Maybe we should do it here and leave it in alloc_can_err_skb() ...
>>> In my at91 driver I don't need the memset, because I unconditionally
>>> read the two 32 bit registers and write it into the ->data of the can_frame.
>>>
>>>>>> +
>>>>>> + return skb;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(alloc_can_skb);
>>>>>> +struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct
>>>>>> can_frame **cf)
>>>>>> +{
>>>>>> + struct sk_buff *skb;
>>>>>> +
>>>>>> + skb = alloc_can_skb(dev, cf);
>>>>>> + if (unlikely(!skb))
>>>>>> + return NULL;
>>>>>> +
>>>>>> + memset(*cf, 0, sizeof(struct can_frame));
>>>> here :-)
>>> Here we need zeroed ->data
>> Ok, but if we think about some improvement, i would suggest to zero only the
>> 8
>> bytes data[] and leave the rest as-is.
>
>> Maybe by setting the u64 casted data[] to zero.
>
> yes, but we might still leak the 3 padding bytes...
>
>>>>>> + (*cf)->can_id = CAN_ERR_FLAG;
>>>>>> + (*cf)->can_dlc = CAN_ERR_DLC;
>> As you can see, the can_id and the can_dlc is written anyway.
>
>> When data[] is uninitialized, we expose 8-11 Bytes of uninitialized
>> contiguous
>> data to the userspace, when can_dlc=0 - i don't know if this is relevant.
>
> In the can error frame use case we need data[] to be zero, because
> usually bits are or'ed into the data[].
>
> When looking at the normal rx path the driver might not need it....(as
> the at91), unconditionally cleaning everything might make other drivers
> simpler.
>
> I doubt that we can measure the performance impact if we generally
> memset the whole can frame, for both error and normal frames. So make it
> clean ;)
Yep, I vote for that as well.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
